You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stas Bekman <st...@stason.org> on 2004/09/27 01:45:24 UTC
segfault ap_save_brigage in the latest 2.0
I get the following segfault with the latest cvs 2.0 branch:
#0 0x080eaed2 in ap_save_brigade (f=0x9541448, saveto=0x9541488,
b=0xbfffebd4, p=0x9550ba8) at util_filter.c:539
539 APR_BRIGADE_CONCAT(*saveto, *b);
Here is the relevant part of the trace:
gdb) bt
#0 0x080eaed2 in ap_save_brigade (f=0x9541448, saveto=0x9541488,
b=0xbfffebd4, p=0x9550ba8) at util_filter.c:539
#1 0x080f5339 in core_output_filter (f=0x9541448, b=0x9550be0) at core.c:4173
#2 0x080eae13 in ap_pass_brigade (next=0x9541448, bb=0x9554e78)
at util_filter.c:511
#3 0x080a7225 in ap_http_header_filter (f=0x9549e38, b=0x9554e78)
at http_protocol.c:1690
#4 0x080eae13 in ap_pass_brigade (next=0x9549e38, bb=0x9554e78)
at util_filter.c:511
#5 0x080edb4f in ap_content_length_filter (f=0x9549e20, b=0x9554e78)
at protocol.c:1256
#6 0x080eae13 in ap_pass_brigade (next=0x9549e20, bb=0x9554e78)
at util_filter.c:511
#7 0x080a8c93 in ap_byterange_filter (f=0x9549e08, bb=0x9554e78)
at http_protocol.c:2875
#8 0x080eae13 in ap_pass_brigade (next=0x9549e08, bb=0x9554e78)
at util_filter.c:511
#9 0x4044dd75 in myfunc (wb=0x9552bb0,
buf=0x9552bb4 "1..10\n# Running u... ", len=1455)
at foo.c:223
myfunc does:
bucket = apr_bucket_transient_create(buf, len, ba);
bb = apr_brigade_create(wb->pool, ba);
APR_BRIGADE_INSERT_TAIL(bb, bucket);
ap_pass_brigade(f, bb);
(gdb) up 9
(gdb) info locals
ba = (apr_bucket_alloc_t *) 0x9544f78
bb = (apr_bucket_brigade *) 0x9554e78
bucket = (apr_bucket *) 0x9544f98
__func__ = "myfunc"
(gdb) source /home/stas/apache.org/httpd-2.0/.gdbinit
(gdb) dump_bucket bucket
bucket=HEAP (0x09544f98) length=1455 data=0x09545178
contents=[1..10~# Running u...] rc=1
(gdb) dump_brigade bb
dump of brigade 0x9554e78
| type (address) | length | data addr | contents
| rc
--------------------------------------------------------------------------------
brigade is empty
end of brigade
(gdb) down 9
#0 0x080eaed2 in ap_save_brigade (f=0x9541448, saveto=0x9541488,
b=0xbfffebd4, p=0x9550ba8) at util_filter.c:539
539 APR_BRIGADE_CONCAT(*saveto, *b);
(gdb) p **saveto
$1 = {p = 0x203a6c61, list = {next = 0x206e7553, prev = 0x20706553},
bucket_alloc = 0x31203632}
(gdb) p **b
$2 = {p = 0x20353030, list = {next = 0x20726f66, prev = 0x756e696c},
bucket_alloc = 0x20230a78}
(gdb) p (**b)->list
$3 = {next = 0x20726f66, prev = 0x756e696c}
(gdb) p (**saveto)->list
$4 = {next = 0x206e7553, prev = 0x20706553}
(gdb) dump_brigade b
dump of brigade 0xbfffebd4
| type (address) | length | data addr | contents
| rc
--------------------------------------------------------------------------------
0 | § (0xbfffebf8) | 156585592 | 0x405cbc18 | [**unknown**] | n/a
1 | ÐÎl
(0xbfffec48) | 156585592 | 0x404edd4e | [**unknown**] | n/a
2 | ÐÎl
(0xbfffec78) | 156585592 | 0x00000004 | [**unknown**] | n/a
3 | ¨ (0xbfffecc8) | 156585592 | 0x080ead44 | [**unknown**] | n/a
4 | ¨ (0xbfffecf8) | 156585592 | 0xbfffed9c | [**unknown**] | n/a
5 |#
(0xbfffed68) | 156585592 | 0x00000000 | [**unknown**] | n/a
6 |#
(0xbfffed98) | 156585592 | 0x4044d9fd | [**unknown**] | n/a
7 | Cannot access memory at address 0x0
One important note is that it doesn't happen when run first on the server
startup, but later on after some other requests, so it must be some
earlier state that affects the later runs.
Any help with this one? Thanks!
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Yes, I have seen exactly the problem you are describing hapenning, and
getting away without pool userdata would have fixed it for me.
After just a cursory looking over this patch (not tested or anything)
it looks good.
Joe Schaefer wrote:
> Index: xs/APR/Pool/APR__Pool.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
> retrieving revision 1.17
> diff -u -r1.17 APR__Pool.h
> --- xs/APR/Pool/APR__Pool.h 14 Jul 2004 23:15:01 -0000 1.17
> +++ xs/APR/Pool/APR__Pool.h 28 Sep 2004 21:32:32 -0000
> @@ -17,6 +17,7 @@
>
> typedef struct {
> SV *sv;
> + PerlInterpreter *perl;
> } mpxs_pool_account_t;
>
> /* XXX: this implementation has a problem with perl ithreads. if a
> @@ -50,26 +51,10 @@
> static MP_INLINE apr_status_t
> mpxs_apr_pool_cleanup(void *cleanup_data)
> {
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW,
> - (apr_pool_t *)cleanup_data);
> - if (!(data && data->sv)) {
> - /* if there is no data, there is nothing to unset */
> - MP_POOL_TRACE(MP_FUNC, "this pool seems to be destroyed already");
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC,
> - "pool 0x%lx contains a valid sv 0x%lx, invalidating it",
> - (unsigned long)data->sv, (unsigned long)cleanup_data);
> -
> - /* invalidate all Perl objects referencing this sv */
> - SvIVX(data->sv) = 0;
> -
> - /* invalidate the reference stored in the pool */
> - data->sv = NULL;
> - /* data->sv will go away by itself when all objects will go away */
> - }
> -
> + mpxs_pool_account_t *acct = cleanup_data;
> + dTHXa(acct->perl);
> + mg_free(acct->sv);
> + SvIVX(acct->sv) = 0;
> return APR_SUCCESS;
> }
>
> @@ -116,9 +101,6 @@
> * mess, trying to destroy an already destroyed pool or even worse
> * a pool allocate in the place of the old one.
> */
> - apr_pool_cleanup_register(child_pool, (void *)child_pool,
> - mpxs_apr_pool_cleanup,
> - apr_pool_cleanup_null);
> #if APR_POOL_DEBUG
> /* child <-> parent <-> ... <-> top ancestry traversal */
> {
> @@ -139,17 +121,22 @@
> #endif
>
> {
> - mpxs_pool_account_t *data =
> - (mpxs_pool_account_t *)apr_pcalloc(child_pool, sizeof(*data));
> -
> SV *rv = sv_setref_pv(NEWSV(0, 0), "APR::Pool", (void*)child_pool);
> + SV *sv = SvRV(rv);
> + mpxs_pool_account_t *acct = apr_palloc(child_pool, sizeof *acct);
>
> - data->sv = SvRV(rv);
> + acct->sv = sv;
> + acct->perl = aTHX;
> +
> + sv_magic(sv, Nullsv, PERL_MAGIC_ext, "APR::Pool", sizeof("APR::Pool"));
>
> - MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
> - (unsigned long)child_pool, data->sv, rv);
>
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, child_pool);
> + apr_pool_cleanup_register(child_pool, (void *)acct,
> + mpxs_apr_pool_cleanup,
> + apr_pool_cleanup_null);
> +
> + MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
> + (unsigned long)child_pool, sv, rv);
>
> return rv;
> }
> @@ -158,10 +145,10 @@
> static MP_INLINE void mpxs_APR__Pool_clear(pTHX_ SV *obj)
> {
> apr_pool_t *p = mp_xs_sv2_APR__Pool(obj);
> - mpxs_pool_account_t *data;
> + SV *sv = SvRV(obj);
> + mpxs_pool_account_t *acct;
>
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
> - if (!(data && data->sv)) {
> + if (mg_find(sv, PERL_MAGIC_ext) == NULL) {
> MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> (unsigned long)p);
> apr_pool_clear(p);
> @@ -171,20 +158,24 @@
> MP_POOL_TRACE(MP_FUNC,
> "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> (unsigned long)p,
> - (unsigned long)data->sv);
> + (unsigned long)sv);
>
> apr_pool_clear(p);
>
> - /* apr_pool_clear removes all the user data, so we need to restore
> + /* apr_pool_clear removes all the cleanup, so we need to restore
> * it. Since clear triggers mpxs_apr_pool_cleanup call, our
> * object's guts get nuked too, so we need to restore them too */
>
> /* this is sv_setref_pv, but for an existing object */
> - sv_setiv(newSVrv(obj, "APR::Pool"), PTR2IV((void*)p));
> - data->sv = SvRV(obj);
> + sv_magic(sv, Nullsv, PERL_MAGIC_ext, "APR::Pool", sizeof("APR::Pool"));
> + SvIVX(sv) = (IV)p;
> + acct = apr_palloc(p, sizeof *acct);
> + acct->sv = sv;
> + acct->perl = aTHX;
>
> - /* reinstall the user data */
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, p);
> + apr_pool_cleanup_register(p, (void *)acct,
> + mpxs_apr_pool_cleanup,
> + apr_pool_cleanup_null);
> }
>
>
> @@ -294,30 +285,7 @@
> apr_pool_t *parent_pool = apr_pool_parent_get(child_pool);
>
> if (parent_pool) {
> - /* ideally this should be done by mp_xs_APR__Pool_2obj. Though
> - * since most of the time we don't use custom pools, we don't
> - * want the overhead of reading and writing pool's userdata in
> - * the general case. therefore we do it here and in
> - * mpxs_apr_pool_create. Though if there are any other
> - * functions, that return perl objects whose guts include a
> - * reference to a custom pool, they must do the ref-counting
> - * as well.
> - */
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, parent_pool);
> - if (data && data->sv) {
> - MP_POOL_TRACE(MP_FUNC,
> - "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> - (unsigned long)parent_pool,
> - (unsigned long)data->sv);
> -
> - return newRV_inc(data->sv);
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> - (unsigned long)parent_pool);
> - return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> - }
> + return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> }
> else {
> MP_POOL_TRACE(MP_FUNC, "pool (0x%lx) has no parents",
> @@ -335,11 +303,18 @@
> apr_pool_t *p;
> SV *sv = SvRV(obj);
>
> + p = mpxs_sv_object_deref(obj, apr_pool_t);
> +
> + if (mg_find(sv, PERL_MAGIC_ext))
> + apr_pool_destroy(p);
> +
> +#if 0
> +
> /* MP_POOL_TRACE(MP_FUNC, "DESTROY 0x%lx-0x%lx", */
> /* (unsigned long)obj,(unsigned long)sv); */
> /* do_sv_dump(0, Perl_debug_log, obj, 0, 4, FALSE, 0); */
>
> - p = mpxs_sv_object_deref(obj, apr_pool_t);
> +
> if (!p) {
> /* non-custom pool */
> MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: not a custom pool");
> @@ -367,5 +342,7 @@
> SvREFCNT(sv));
> }
> }
> +
> +#endif
> }
>
>
>
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
> + # the APR_EGENERAL error string changed for APR 1.0
> + my $egeneral = "Error string not specified yet";
> + $egeneral = "Internal error"
> + if need_min_apache_version('2.1.0');
this should be have_min_apache_version() instead. basically, use the need*
variants in calls to plan() and have* variants everywhere else, as need*
adds to the global test skip message, which can leak across tests.
other than that, looks great :)
I'm looking into the content_length failure now. what I think is happening
there is that we coded it as httpd behaved at the moment, but that something
has recently changed over there and I'm just now catching up on my email.
as an aside, I think we kind of informally agreed that the tests in t/apache
were for testing internal apache things in an attempt to let us know when
some basic httpd functionality changed that we should be knowing about. so,
in this case, that the behavior of a handler with a zero content length has
changed wrt the C-L header isn't as important to mod_perl as it is to us
understanding things when users report problems (which was the reason we
wrote this particular test IIRC).
anyway, I'm building against current cvs now after a few days of travelling,
so I hope to have a solution for that test soon.
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Geoffrey Young <ge...@modperlcookbook.org>.
Joe Orton wrote:
> On Wed, Sep 29, 2004 at 11:43:27PM -0400, Stas Bekman wrote:
>
>>Joe Schaefer wrote:
>>
>>>>>t/api/aplog.t 35 1 2.86% 24
>
>
> I wrote a fix for this one the other day since it was easy.
>
committed, thanks!
--Geoff
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
On Wed, Sep 29, 2004 at 11:43:27PM -0400, Stas Bekman wrote:
> Joe Schaefer wrote:
> >>>t/api/aplog.t 35 1 2.86% 24
I wrote a fix for this one the other day since it was easy.
--- response/TestAPI/aplog.pm 24 Sep 2004 19:55:35 -0000 1.21
+++ response/TestAPI/aplog.pm 28 Sep 2004 12:19:36 -0000
@@ -74,11 +74,16 @@
qr/: log_serror test 1$/m,
'$s->log_serror(__FILE__, __LINE__, LOG_DEBUG...)';
+ # the APR_EGENERAL error string changed for APR 1.0
+ my $egeneral = "Error string not specified yet";
+ $egeneral = "Internal error"
+ if need_min_apache_version('2.1.0');
+
t_server_log_warn_is_expected();
$s->log_serror(Apache::Log::LOG_MARK, Apache::LOG_DEBUG,
APR::EGENERAL, "log_serror test 2");
ok t_cmp $logdiff->diff,
- qr/Error string not specified yet: log_serror test 2/,
+ qr/$egeneral: log_serror test 2/,
'$s->log_serror(LOG_MARK, LOG_DEBUG, APR::EGENERAL...)';
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
>
>>Joe Schaefer wrote:
>>
>>>... That doesn't mean all the tests pass,
>>>Failed Test Stat Wstat Total Fail Failed List of Failed
>>>-------------------------------------------------------------------------------
>>>t/apache/content_length_header.t 27 3 11.11% 2 5 17
>>>t/api/aplog.t 35 1 2.86% 24
>>>4 tests skipped.
>>>Failed 2/219 test scripts, 99.09% okay. 4/4208 subtests failed, 99.90% okay.
>>
>>But what about the verbose run and the error_log and the usual t/REPORT output?
>
>
> I'm fairly certain I've posted those previously, but if not, it's
> no big deal since I know what's going on there.
You mean you know the cause for these failures and you will post a fix as
well. I just can't tell what the problem is, since they don't fail for me.
But if you are handling those, then no need to post anything extra :)
Thanks Joe.
> However, you've
> missed my point, which was that the above is result is absolutely
> wonderful, given a week's worth of results like this one,
Excellent, I didn't miss your point, just thought you needed help to get
100% pass :)
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Joe Schaefer wrote:
> >... That doesn't mean all the tests pass,
> > Failed Test Stat Wstat Total Fail Failed List of Failed
> > -------------------------------------------------------------------------------
> > t/apache/content_length_header.t 27 3 11.11% 2 5 17
> > t/api/aplog.t 35 1 2.86% 24
> > 4 tests skipped.
> > Failed 2/219 test scripts, 99.09% okay. 4/4208 subtests failed, 99.90% okay.
>
> But what about the verbose run and the error_log and the usual t/REPORT output?
I'm fairly certain I've posted those previously, but if not, it's
no big deal since I know what's going on there. However, you've
missed my point, which was that the above is result is absolutely
wonderful, given a week's worth of results like this one, generated
from today's mp2/httpd/apr cvs:
Failed Test Stat Wstat Total Fail Failed List of Failed
-------------------------------------------------------------------------------
t/apache/content_length_header.t 27 3 11.11% 2 5 17
t/api/aplog.t 35 1 2.86% 24
t/filter/both_str_req_add.t 1 1 100.00% 1
t/filter/out_bbs_filebucket.t 10 2 20.00% 1-2
t/modules/proxy.t 255 65280 1 2 200.00% 1
t/perl/ithreads.t 4 3 75.00% 2-4
2 tests skipped.
Failed 6/218 test scripts, 97.25% okay. 11/4198 subtests failed, 99.74% okay.
[warning] server localhost.localdomain:9500 shutdown
[warning] port 9500 still in use...
done
[ error] error running tests (please examine t/logs/error_log)
[ info] an old core file has been found: /home/smoke/src/apache/modperl-2.0/t/core.22806
[ info] an old core file has been found: /home/smoke/src/apache/modperl-2.0/t/core.25623
[ info] an old core file has been found: /home/smoke/src/apache/modperl-2.0/t/core.31268
[ info] an old core file has been found: /home/smoke/src/apache/modperl-2.0/t/core.2722
[ error] oh crap, server dumped core
[ error] for stacktrace, run: gdb /home/smoke/apache/httpd-worker/bin/httpd -core /home/smoke/src/apache/modperl-2.0/t/core.9756
+--------------------------------------------------------+
| Please file a bug report: http://perl.apache.org/bugs/ |
+--------------------------------------------------------+
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
>... That doesn't mean all the tests pass,
>
> Failed Test Stat Wstat Total Fail Failed List of Failed
> -------------------------------------------------------------------------------
> t/apache/content_length_header.t 27 3 11.11% 2 5 17
> t/api/aplog.t 35 1 2.86% 24
> 4 tests skipped.
> Failed 2/219 test scripts, 99.09% okay. 4/4208 subtests failed, 99.90% okay.
But what about the verbose run and the error_log and the usual t/REPORT
output?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
> Joe Schaefer wrote:
>
> > Looks good. It tests fine, so I tried to include it in the patch,
> > but alas:
> > $ cvs add pool_lifetime.t
> > cvs [server aborted]: "add" requires write access to the repository
>
> Right, but you should run:
>
> perl util/getdiff.pl newfile1 newfile2 ...
Thanks. The good news (for me anyway) is that with the combined
patches I posted today, I'm once again able to complete the test
suite without segfaults- something I haven't been able to do reliably
for over a week now. That doesn't mean all the tests pass,
Failed Test Stat Wstat Total Fail Failed List of Failed
-------------------------------------------------------------------------------
t/apache/content_length_header.t 27 3 11.11% 2 5 17
t/api/aplog.t 35 1 2.86% 24
4 tests skipped.
Failed 2/219 test scripts, 99.09% okay. 4/4208 subtests failed, 99.90% okay.
but at least the flaky behavior that have been me over
the last two weeks is gone for now, assuming the patches
are ok with you guys. If not, let me know and I'll fix+
post them again...
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Looks good. It tests fine, so I tried to include it
> in the patch, but alas:
>
> $ cvs add pool_lifetime.t
> cvs [server aborted]: "add" requires write access to the repository
Right, but you should run:
perl util/getdiff.pl newfile1 newfile2 ...
and it'll create a diff for new files against /dev/null and give the
normal diff for files which are already under the cvs control.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philippe M. Chiasson" <go...@ectoplasm.org> writes:
> Looks good, I am going to give it a spin shortly. Only one note
> after looking over this patch. I'd keep the definition of
> mpxs_pool_account_t at the top
>
> typedef struct {
> SV *sv;
> #ifdef USE_ITHREADS
> PerlInterpreter *perl;
> modperl_interp_t *interp;
> #endif
> } mpxs_pool_account_t;
>
+1.
[...]
> > +#define MP_APR_POOL_SV_DROPS_OWNERSHIP(acct) do { \
> > + dTHXa(acct->perl); \
> > + mg_free(acct->sv); \
> > + SvIVX(acct->sv) = 0; \
> > + if (modperl_opt_interp_unselect && acct->interp) { \
> > + /* this will decrement the interp refcnt until \
> > + * there are no more references, in which case \
> > + * the interpreter will be putback into the mip \
> > + */ \
> > + (void)modperl_opt_interp_unselect(acct->interp); \
> > + } \
> > +} while (0)
> > +
> > +
> > +#ifdef USE_ITHREADS
Oops- MP_APR_POOL_SV_DROPS_OWNERSHIP also needs to be inside an
#ifdef USE_ITHREADS
because of the acct->interp call. The non-ithreads version should
probably look like this:
#define MP_APR_POOL_SV_DROPS_OWNERSHIP(acct) do { \
mg_free(acct->sv); \
SvIVX(acct->sv) = 0; \
} while (0)
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Joe Schaefer wrote:
> "Philippe M. Chiasson" <go...@ectoplasm.org> writes:
>
>
>>I've tested it hapilly and it is both a simpler implementation
>>and gets away with quite an annoying bug, so I'd like to see
>>it integrated with a few tweaks.
>
>
> OK, I've cleaned it up with a few macros as you
> suggested. Patch below...
>
>
>>- At least one test case for this (check my post
>>http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=109596770111457&w=2
>
>
> Looks good. It tests fine, so I tried to include it
> in the patch, but alas:
>
> $ cvs add pool_lifetime.t
> cvs [server aborted]: "add" requires write access to the repository
Looks good, I am going to give it a spin shortly. Only one note after
looking over this patch. I'd keep the definition of mpxs_pool_account_t
at the top
typedef struct {
SV *sv;
#ifdef USE_ITHREADS
PerlInterpreter *perl;
modperl_interp_t *interp;
#endif
} mpxs_pool_account_t;
instead of having to completely different and separated definitions. It's
a small change, but I think it clarify things a little bit. Apart from that,
patch does look quite good.
>
> Index: xs/APR/Pool/APR__Pool.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
> retrieving revision 1.17
> diff -u -r1.17 APR__Pool.h
> --- xs/APR/Pool/APR__Pool.h 14 Jul 2004 23:15:01 -0000 1.17
> +++ xs/APR/Pool/APR__Pool.h 30 Sep 2004 01:14:04 -0000
> @@ -15,10 +15,6 @@
>
> #define MP_APR_POOL_NEW "APR::Pool::new"
>
> -typedef struct {
> - SV *sv;
> -} mpxs_pool_account_t;
> -
> /* XXX: this implementation has a problem with perl ithreads. if a
> * custom pool is allocated, and then a thread is spawned we now have
> * two copies of the pool object, each living in a different perl
> @@ -33,6 +29,82 @@
> * that?) may be we can skip those?
> */
>
> +#ifndef MP_SOURCE_SCAN
> +#include "apr_optional.h"
> +static
> +APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *modperl_opt_interp_unselect;
> +#endif
> +
> +
> +#define MP_APR_POOL_SV_HAS_OWNERSHIP(sv) (mg_find(sv, PERL_MAGIC_ext) != NULL)
> +
> +#define MP_APR_POOL_SV_DROPS_OWNERSHIP(acct) do { \
> + dTHXa(acct->perl); \
> + mg_free(acct->sv); \
> + SvIVX(acct->sv) = 0; \
> + if (modperl_opt_interp_unselect && acct->interp) { \
> + /* this will decrement the interp refcnt until \
> + * there are no more references, in which case \
> + * the interpreter will be putback into the mip \
> + */ \
> + (void)modperl_opt_interp_unselect(acct->interp); \
> + } \
> +} while (0)
> +
> +
> +#ifdef USE_ITHREADS
> +
> +typedef struct {
> + SV *sv;
> + PerlInterpreter *perl;
> + modperl_interp_t *interp;
> +} mpxs_pool_account_t;
> +
> +
> +#define MP_APR_POOL_SV_TAKES_OWNERSHIP(SV, P) do { \
> + mpxs_pool_account_t *acct = apr_palloc(P, sizeof *acct); \
> + acct->sv = SV; \
> + acct->perl = aTHX; \
> + SvIVX(SV) = PTR2IV(P); \
> + \
> + sv_magic(SV, Nullsv, PERL_MAGIC_ext, \
> + MP_APR_POOL_NEW, sizeof(MP_APR_POOL_NEW)); \
> + \
> + apr_pool_cleanup_register(P, (void *)acct, \
> + mpxs_apr_pool_cleanup, \
> + apr_pool_cleanup_null); \
> + \
> + /* make sure interpreter is not putback into the mip \
> + * until this cleanup has run. \
> + */ \
> + if ((acct->interp = MP_THX_INTERP_GET(aTHX))) { \
> + acct->interp->refcnt++; \
> + } \
> +} while (0)
> +
> +#else
> +
> +typedef struct {
> + SV *sv;
> +} mpxs_pool_account_t;
> +
> +
> +#define MP_APR_POOL_SV_TAKES_OWNERSHIP(SV, P) do { \
> + mpxs_pool_account_t *acct = apr_palloc(P, sizeof *acct); \
> + acct->sv = SV; \
> + SvIVX(SV) = PTR2IV(P); \
> + \
> + sv_magic(SV, Nullsv, PERL_MAGIC_ext, \
> + MP_APR_POOL_NEW, sizeof(MP_APR_POOL_NEW)); \
> + \
> + apr_pool_cleanup_register(P, (void *)acct, \
> + mpxs_apr_pool_cleanup, \
> + apr_pool_cleanup_null); \
> +} while (0)
> +
> +#endif
> +
> +
> /* XXX: should we make it a new global tracing category
> * MOD_PERL_TRACE=p for tracing pool management? */
> #define MP_POOL_TRACE_DO 0
> @@ -50,26 +122,8 @@
> static MP_INLINE apr_status_t
> mpxs_apr_pool_cleanup(void *cleanup_data)
> {
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW,
> - (apr_pool_t *)cleanup_data);
> - if (!(data && data->sv)) {
> - /* if there is no data, there is nothing to unset */
> - MP_POOL_TRACE(MP_FUNC, "this pool seems to be destroyed already");
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC,
> - "pool 0x%lx contains a valid sv 0x%lx, invalidating it",
> - (unsigned long)data->sv, (unsigned long)cleanup_data);
> -
> - /* invalidate all Perl objects referencing this sv */
> - SvIVX(data->sv) = 0;
> -
> - /* invalidate the reference stored in the pool */
> - data->sv = NULL;
> - /* data->sv will go away by itself when all objects will go away */
> - }
> -
> + mpxs_pool_account_t *acct = cleanup_data;
> + MP_APR_POOL_SV_DROPS_OWNERSHIP(acct);
MP_APR_POOL_SV_DROPS_OWNERSHIP((mpxs_pool_account_t *)cleanup_data)
> return APR_SUCCESS;
> }
>
> @@ -116,9 +170,6 @@
> * mess, trying to destroy an already destroyed pool or even worse
> * a pool allocate in the place of the old one.
> */
> - apr_pool_cleanup_register(child_pool, (void *)child_pool,
> - mpxs_apr_pool_cleanup,
> - apr_pool_cleanup_null);
> #if APR_POOL_DEBUG
> /* child <-> parent <-> ... <-> top ancestry traversal */
> {
> @@ -139,17 +190,13 @@
> #endif
>
> {
> - mpxs_pool_account_t *data =
> - (mpxs_pool_account_t *)apr_pcalloc(child_pool, sizeof(*data));
> -
> SV *rv = sv_setref_pv(NEWSV(0, 0), "APR::Pool", (void*)child_pool);
> + SV *sv = SvRV(rv);
>
> - data->sv = SvRV(rv);
> + MP_APR_POOL_SV_TAKES_OWNERSHIP(sv, child_pool);
>
> MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
> - (unsigned long)child_pool, data->sv, rv);
> -
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, child_pool);
> + (unsigned long)child_pool, sv, rv);
>
> return rv;
> }
> @@ -158,10 +205,9 @@
> static MP_INLINE void mpxs_APR__Pool_clear(pTHX_ SV *obj)
> {
> apr_pool_t *p = mp_xs_sv2_APR__Pool(obj);
> - mpxs_pool_account_t *data;
> + SV *sv = SvRV(obj);
>
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
> - if (!(data && data->sv)) {
> + if (!MP_APR_POOL_SV_HAS_OWNERSHIP(sv)) {
> MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> (unsigned long)p);
> apr_pool_clear(p);
> @@ -171,20 +217,15 @@
> MP_POOL_TRACE(MP_FUNC,
> "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> (unsigned long)p,
> - (unsigned long)data->sv);
> + (unsigned long)sv);
>
> apr_pool_clear(p);
>
> - /* apr_pool_clear removes all the user data, so we need to restore
> + /* apr_pool_clear removes all the cleanups, so we need to restore
> * it. Since clear triggers mpxs_apr_pool_cleanup call, our
> * object's guts get nuked too, so we need to restore them too */
>
> - /* this is sv_setref_pv, but for an existing object */
> - sv_setiv(newSVrv(obj, "APR::Pool"), PTR2IV((void*)p));
> - data->sv = SvRV(obj);
> -
> - /* reinstall the user data */
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, p);
> + MP_APR_POOL_SV_TAKES_OWNERSHIP(sv, p);
> }
>
>
> @@ -203,11 +244,6 @@
> * @param data internal storage
> */
>
> -#ifndef MP_SOURCE_SCAN
> -#include "apr_optional.h"
> -static
> -APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *modperl_opt_interp_unselect;
> -#endif
>
> static apr_status_t mpxs_cleanup_run(void *data)
> {
> @@ -294,30 +330,7 @@
> apr_pool_t *parent_pool = apr_pool_parent_get(child_pool);
>
> if (parent_pool) {
> - /* ideally this should be done by mp_xs_APR__Pool_2obj. Though
> - * since most of the time we don't use custom pools, we don't
> - * want the overhead of reading and writing pool's userdata in
> - * the general case. therefore we do it here and in
> - * mpxs_apr_pool_create. Though if there are any other
> - * functions, that return perl objects whose guts include a
> - * reference to a custom pool, they must do the ref-counting
> - * as well.
> - */
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, parent_pool);
> - if (data && data->sv) {
> - MP_POOL_TRACE(MP_FUNC,
> - "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> - (unsigned long)parent_pool,
> - (unsigned long)data->sv);
> -
> - return newRV_inc(data->sv);
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> - (unsigned long)parent_pool);
> - return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> - }
> + return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> }
> else {
> MP_POOL_TRACE(MP_FUNC, "pool (0x%lx) has no parents",
> @@ -330,42 +343,14 @@
> * destroy a pool
> * @param obj an APR::Pool object
> */
> +
> static MP_INLINE void mpxs_apr_pool_DESTROY(pTHX_ SV *obj)
> {
> - apr_pool_t *p;
> SV *sv = SvRV(obj);
>
> - /* MP_POOL_TRACE(MP_FUNC, "DESTROY 0x%lx-0x%lx", */
> - /* (unsigned long)obj,(unsigned long)sv); */
> - /* do_sv_dump(0, Perl_debug_log, obj, 0, 4, FALSE, 0); */
> -
> - p = mpxs_sv_object_deref(obj, apr_pool_t);
> - if (!p) {
> - /* non-custom pool */
> - MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: not a custom pool");
> - return;
> - }
> -
> - if (sv && SvOK(sv)) {
> - mpxs_pool_account_t *data;
> -
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
> - if (!(data && data->sv)) {
> - MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: no sv found");
> - return;
> - }
> -
> - if (SvREFCNT(sv) == 1) {
> - MP_POOL_TRACE(MP_FUNC, "call apr_pool_destroy: last reference");
> - apr_pool_destroy(p);
> - }
> - else {
> - /* when the pool object dies, sv's ref count decrements
> - * itself automatically */
> - MP_POOL_TRACE(MP_FUNC,
> - "skip apr_pool_destroy: refcount > 1 (%d)",
> - SvREFCNT(sv));
> - }
> + if (MP_APR_POOL_SV_HAS_OWNERSHIP(sv)) {
> + apr_pool_t *p = mpxs_sv_object_deref(obj, apr_pool_t);
> + apr_pool_destroy(p);
> }
> }
>
>
>
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philippe M. Chiasson" <go...@ectoplasm.org> writes:
> I've tested it hapilly and it is both a simpler implementation
> and gets away with quite an annoying bug, so I'd like to see
> it integrated with a few tweaks.
OK, I've cleaned it up with a few macros as you
suggested. Patch below...
> - At least one test case for this (check my post
> http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=109596770111457&w=2
Looks good. It tests fine, so I tried to include it
in the patch, but alas:
$ cvs add pool_lifetime.t
cvs [server aborted]: "add" requires write access to the repository
Index: xs/APR/Pool/APR__Pool.h
===================================================================
RCS file: /home/cvspublic/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
retrieving revision 1.17
diff -u -r1.17 APR__Pool.h
--- xs/APR/Pool/APR__Pool.h 14 Jul 2004 23:15:01 -0000 1.17
+++ xs/APR/Pool/APR__Pool.h 30 Sep 2004 01:14:04 -0000
@@ -15,10 +15,6 @@
#define MP_APR_POOL_NEW "APR::Pool::new"
-typedef struct {
- SV *sv;
-} mpxs_pool_account_t;
-
/* XXX: this implementation has a problem with perl ithreads. if a
* custom pool is allocated, and then a thread is spawned we now have
* two copies of the pool object, each living in a different perl
@@ -33,6 +29,82 @@
* that?) may be we can skip those?
*/
+#ifndef MP_SOURCE_SCAN
+#include "apr_optional.h"
+static
+APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *modperl_opt_interp_unselect;
+#endif
+
+
+#define MP_APR_POOL_SV_HAS_OWNERSHIP(sv) (mg_find(sv, PERL_MAGIC_ext) != NULL)
+
+#define MP_APR_POOL_SV_DROPS_OWNERSHIP(acct) do { \
+ dTHXa(acct->perl); \
+ mg_free(acct->sv); \
+ SvIVX(acct->sv) = 0; \
+ if (modperl_opt_interp_unselect && acct->interp) { \
+ /* this will decrement the interp refcnt until \
+ * there are no more references, in which case \
+ * the interpreter will be putback into the mip \
+ */ \
+ (void)modperl_opt_interp_unselect(acct->interp); \
+ } \
+} while (0)
+
+
+#ifdef USE_ITHREADS
+
+typedef struct {
+ SV *sv;
+ PerlInterpreter *perl;
+ modperl_interp_t *interp;
+} mpxs_pool_account_t;
+
+
+#define MP_APR_POOL_SV_TAKES_OWNERSHIP(SV, P) do { \
+ mpxs_pool_account_t *acct = apr_palloc(P, sizeof *acct); \
+ acct->sv = SV; \
+ acct->perl = aTHX; \
+ SvIVX(SV) = PTR2IV(P); \
+ \
+ sv_magic(SV, Nullsv, PERL_MAGIC_ext, \
+ MP_APR_POOL_NEW, sizeof(MP_APR_POOL_NEW)); \
+ \
+ apr_pool_cleanup_register(P, (void *)acct, \
+ mpxs_apr_pool_cleanup, \
+ apr_pool_cleanup_null); \
+ \
+ /* make sure interpreter is not putback into the mip \
+ * until this cleanup has run. \
+ */ \
+ if ((acct->interp = MP_THX_INTERP_GET(aTHX))) { \
+ acct->interp->refcnt++; \
+ } \
+} while (0)
+
+#else
+
+typedef struct {
+ SV *sv;
+} mpxs_pool_account_t;
+
+
+#define MP_APR_POOL_SV_TAKES_OWNERSHIP(SV, P) do { \
+ mpxs_pool_account_t *acct = apr_palloc(P, sizeof *acct); \
+ acct->sv = SV; \
+ SvIVX(SV) = PTR2IV(P); \
+ \
+ sv_magic(SV, Nullsv, PERL_MAGIC_ext, \
+ MP_APR_POOL_NEW, sizeof(MP_APR_POOL_NEW)); \
+ \
+ apr_pool_cleanup_register(P, (void *)acct, \
+ mpxs_apr_pool_cleanup, \
+ apr_pool_cleanup_null); \
+} while (0)
+
+#endif
+
+
/* XXX: should we make it a new global tracing category
* MOD_PERL_TRACE=p for tracing pool management? */
#define MP_POOL_TRACE_DO 0
@@ -50,26 +122,8 @@
static MP_INLINE apr_status_t
mpxs_apr_pool_cleanup(void *cleanup_data)
{
- mpxs_pool_account_t *data;
- apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW,
- (apr_pool_t *)cleanup_data);
- if (!(data && data->sv)) {
- /* if there is no data, there is nothing to unset */
- MP_POOL_TRACE(MP_FUNC, "this pool seems to be destroyed already");
- }
- else {
- MP_POOL_TRACE(MP_FUNC,
- "pool 0x%lx contains a valid sv 0x%lx, invalidating it",
- (unsigned long)data->sv, (unsigned long)cleanup_data);
-
- /* invalidate all Perl objects referencing this sv */
- SvIVX(data->sv) = 0;
-
- /* invalidate the reference stored in the pool */
- data->sv = NULL;
- /* data->sv will go away by itself when all objects will go away */
- }
-
+ mpxs_pool_account_t *acct = cleanup_data;
+ MP_APR_POOL_SV_DROPS_OWNERSHIP(acct);
return APR_SUCCESS;
}
@@ -116,9 +170,6 @@
* mess, trying to destroy an already destroyed pool or even worse
* a pool allocate in the place of the old one.
*/
- apr_pool_cleanup_register(child_pool, (void *)child_pool,
- mpxs_apr_pool_cleanup,
- apr_pool_cleanup_null);
#if APR_POOL_DEBUG
/* child <-> parent <-> ... <-> top ancestry traversal */
{
@@ -139,17 +190,13 @@
#endif
{
- mpxs_pool_account_t *data =
- (mpxs_pool_account_t *)apr_pcalloc(child_pool, sizeof(*data));
-
SV *rv = sv_setref_pv(NEWSV(0, 0), "APR::Pool", (void*)child_pool);
+ SV *sv = SvRV(rv);
- data->sv = SvRV(rv);
+ MP_APR_POOL_SV_TAKES_OWNERSHIP(sv, child_pool);
MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
- (unsigned long)child_pool, data->sv, rv);
-
- apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, child_pool);
+ (unsigned long)child_pool, sv, rv);
return rv;
}
@@ -158,10 +205,9 @@
static MP_INLINE void mpxs_APR__Pool_clear(pTHX_ SV *obj)
{
apr_pool_t *p = mp_xs_sv2_APR__Pool(obj);
- mpxs_pool_account_t *data;
+ SV *sv = SvRV(obj);
- apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
- if (!(data && data->sv)) {
+ if (!MP_APR_POOL_SV_HAS_OWNERSHIP(sv)) {
MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
(unsigned long)p);
apr_pool_clear(p);
@@ -171,20 +217,15 @@
MP_POOL_TRACE(MP_FUNC,
"parent pool (0x%lx) is a custom pool, sv 0x%lx",
(unsigned long)p,
- (unsigned long)data->sv);
+ (unsigned long)sv);
apr_pool_clear(p);
- /* apr_pool_clear removes all the user data, so we need to restore
+ /* apr_pool_clear removes all the cleanups, so we need to restore
* it. Since clear triggers mpxs_apr_pool_cleanup call, our
* object's guts get nuked too, so we need to restore them too */
- /* this is sv_setref_pv, but for an existing object */
- sv_setiv(newSVrv(obj, "APR::Pool"), PTR2IV((void*)p));
- data->sv = SvRV(obj);
-
- /* reinstall the user data */
- apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, p);
+ MP_APR_POOL_SV_TAKES_OWNERSHIP(sv, p);
}
@@ -203,11 +244,6 @@
* @param data internal storage
*/
-#ifndef MP_SOURCE_SCAN
-#include "apr_optional.h"
-static
-APR_OPTIONAL_FN_TYPE(modperl_interp_unselect) *modperl_opt_interp_unselect;
-#endif
static apr_status_t mpxs_cleanup_run(void *data)
{
@@ -294,30 +330,7 @@
apr_pool_t *parent_pool = apr_pool_parent_get(child_pool);
if (parent_pool) {
- /* ideally this should be done by mp_xs_APR__Pool_2obj. Though
- * since most of the time we don't use custom pools, we don't
- * want the overhead of reading and writing pool's userdata in
- * the general case. therefore we do it here and in
- * mpxs_apr_pool_create. Though if there are any other
- * functions, that return perl objects whose guts include a
- * reference to a custom pool, they must do the ref-counting
- * as well.
- */
- mpxs_pool_account_t *data;
- apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, parent_pool);
- if (data && data->sv) {
- MP_POOL_TRACE(MP_FUNC,
- "parent pool (0x%lx) is a custom pool, sv 0x%lx",
- (unsigned long)parent_pool,
- (unsigned long)data->sv);
-
- return newRV_inc(data->sv);
- }
- else {
- MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
- (unsigned long)parent_pool);
- return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
- }
+ return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
}
else {
MP_POOL_TRACE(MP_FUNC, "pool (0x%lx) has no parents",
@@ -330,42 +343,14 @@
* destroy a pool
* @param obj an APR::Pool object
*/
+
static MP_INLINE void mpxs_apr_pool_DESTROY(pTHX_ SV *obj)
{
- apr_pool_t *p;
SV *sv = SvRV(obj);
- /* MP_POOL_TRACE(MP_FUNC, "DESTROY 0x%lx-0x%lx", */
- /* (unsigned long)obj,(unsigned long)sv); */
- /* do_sv_dump(0, Perl_debug_log, obj, 0, 4, FALSE, 0); */
-
- p = mpxs_sv_object_deref(obj, apr_pool_t);
- if (!p) {
- /* non-custom pool */
- MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: not a custom pool");
- return;
- }
-
- if (sv && SvOK(sv)) {
- mpxs_pool_account_t *data;
-
- apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
- if (!(data && data->sv)) {
- MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: no sv found");
- return;
- }
-
- if (SvREFCNT(sv) == 1) {
- MP_POOL_TRACE(MP_FUNC, "call apr_pool_destroy: last reference");
- apr_pool_destroy(p);
- }
- else {
- /* when the pool object dies, sv's ref count decrements
- * itself automatically */
- MP_POOL_TRACE(MP_FUNC,
- "skip apr_pool_destroy: refcount > 1 (%d)",
- SvREFCNT(sv));
- }
+ if (MP_APR_POOL_SV_HAS_OWNERSHIP(sv)) {
+ apr_pool_t *p = mpxs_sv_object_deref(obj, apr_pool_t);
+ apr_pool_destroy(p);
}
}
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Joe Schaefer wrote:
> "Philippe M. Chiasson" <go...@ectoplasm.org> writes:
>
>
>>Forgot to mention I want to test this patch and look
>>thru it carefully before it goes in.
>
>
> Thanks Phillippe! Keep in mind I threw it together rather
> hastily, so if you still like it after testing,
I've tested it hapilly and it is both a simpler implementation
and gets away with quite an annoying bug, so I'd like to see
it integrated with a few tweaks.
> I'll gladly
> clean it up a bit (sync the comments and a the missing
> #ifdef USE_ITHREADS).
Yes, a few notes on that patch:
- Watch lines > 72 charactres
- sv_magic(sv, Nullsv, PERL_MAGIC_ext, "APR::Pool", sizeof("APR::Pool"));
- Probably make it a macro since it's twice in the code
- "APR::Pool" should be in it's own #define
- #ifdef USE_ITHREADS where needed
- At least one test case for this (check my post http://marc.theaimsgroup.com/?l=apache-modperl-dev&m=109596770111457&w=2)
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philippe M. Chiasson" <go...@ectoplasm.org> writes:
> Forgot to mention I want to test this patch and look
> thru it carefully before it goes in.
Thanks Phillippe! Keep in mind I threw it together rather
hastily, so if you still like it after testing, I'll gladly
clean it up a bit (sync the comments and a the missing
#ifdef USE_ITHREADS).
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: [PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Forgot to mention I want to test this patch and look thru it carefully
before it goes in. Won't have time to get to it today, but I'll get around
it tomorrow.
Joe Schaefer wrote:
> ? config.nice
> ? pool.patch
> ? t/core.21302
> Index: xs/APR/Pool/APR__Pool.h
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/xs/APR/Pool/APR__Pool.h,v
> retrieving revision 1.17
> diff -u -r1.17 APR__Pool.h
> --- xs/APR/Pool/APR__Pool.h 14 Jul 2004 23:15:01 -0000 1.17
> +++ xs/APR/Pool/APR__Pool.h 28 Sep 2004 21:32:32 -0000
> @@ -17,6 +17,7 @@
>
> typedef struct {
> SV *sv;
> + PerlInterpreter *perl;
> } mpxs_pool_account_t;
>
> /* XXX: this implementation has a problem with perl ithreads. if a
> @@ -50,26 +51,10 @@
> static MP_INLINE apr_status_t
> mpxs_apr_pool_cleanup(void *cleanup_data)
> {
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW,
> - (apr_pool_t *)cleanup_data);
> - if (!(data && data->sv)) {
> - /* if there is no data, there is nothing to unset */
> - MP_POOL_TRACE(MP_FUNC, "this pool seems to be destroyed already");
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC,
> - "pool 0x%lx contains a valid sv 0x%lx, invalidating it",
> - (unsigned long)data->sv, (unsigned long)cleanup_data);
> -
> - /* invalidate all Perl objects referencing this sv */
> - SvIVX(data->sv) = 0;
> -
> - /* invalidate the reference stored in the pool */
> - data->sv = NULL;
> - /* data->sv will go away by itself when all objects will go away */
> - }
> -
> + mpxs_pool_account_t *acct = cleanup_data;
> + dTHXa(acct->perl);
> + mg_free(acct->sv);
> + SvIVX(acct->sv) = 0;
> return APR_SUCCESS;
> }
>
> @@ -116,9 +101,6 @@
> * mess, trying to destroy an already destroyed pool or even worse
> * a pool allocate in the place of the old one.
> */
> - apr_pool_cleanup_register(child_pool, (void *)child_pool,
> - mpxs_apr_pool_cleanup,
> - apr_pool_cleanup_null);
> #if APR_POOL_DEBUG
> /* child <-> parent <-> ... <-> top ancestry traversal */
> {
> @@ -139,17 +121,22 @@
> #endif
>
> {
> - mpxs_pool_account_t *data =
> - (mpxs_pool_account_t *)apr_pcalloc(child_pool, sizeof(*data));
> -
> SV *rv = sv_setref_pv(NEWSV(0, 0), "APR::Pool", (void*)child_pool);
> + SV *sv = SvRV(rv);
> + mpxs_pool_account_t *acct = apr_palloc(child_pool, sizeof *acct);
>
> - data->sv = SvRV(rv);
> + acct->sv = sv;
> + acct->perl = aTHX;
> +
> + sv_magic(sv, Nullsv, PERL_MAGIC_ext, "APR::Pool", sizeof("APR::Pool"));
>
> - MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
> - (unsigned long)child_pool, data->sv, rv);
>
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, child_pool);
> + apr_pool_cleanup_register(child_pool, (void *)acct,
> + mpxs_apr_pool_cleanup,
> + apr_pool_cleanup_null);
> +
> + MP_POOL_TRACE(MP_FUNC, "sub-pool p: 0x%lx, sv: 0x%lx, rv: 0x%lx",
> + (unsigned long)child_pool, sv, rv);
>
> return rv;
> }
> @@ -158,10 +145,10 @@
> static MP_INLINE void mpxs_APR__Pool_clear(pTHX_ SV *obj)
> {
> apr_pool_t *p = mp_xs_sv2_APR__Pool(obj);
> - mpxs_pool_account_t *data;
> + SV *sv = SvRV(obj);
> + mpxs_pool_account_t *acct;
>
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, p);
> - if (!(data && data->sv)) {
> + if (mg_find(sv, PERL_MAGIC_ext) == NULL) {
> MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> (unsigned long)p);
> apr_pool_clear(p);
> @@ -171,20 +158,24 @@
> MP_POOL_TRACE(MP_FUNC,
> "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> (unsigned long)p,
> - (unsigned long)data->sv);
> + (unsigned long)sv);
>
> apr_pool_clear(p);
>
> - /* apr_pool_clear removes all the user data, so we need to restore
> + /* apr_pool_clear removes all the cleanup, so we need to restore
> * it. Since clear triggers mpxs_apr_pool_cleanup call, our
> * object's guts get nuked too, so we need to restore them too */
>
> /* this is sv_setref_pv, but for an existing object */
> - sv_setiv(newSVrv(obj, "APR::Pool"), PTR2IV((void*)p));
> - data->sv = SvRV(obj);
> + sv_magic(sv, Nullsv, PERL_MAGIC_ext, "APR::Pool", sizeof("APR::Pool"));
> + SvIVX(sv) = (IV)p;
> + acct = apr_palloc(p, sizeof *acct);
> + acct->sv = sv;
> + acct->perl = aTHX;
>
> - /* reinstall the user data */
> - apr_pool_userdata_set(data, MP_APR_POOL_NEW, NULL, p);
> + apr_pool_cleanup_register(p, (void *)acct,
> + mpxs_apr_pool_cleanup,
> + apr_pool_cleanup_null);
> }
>
>
> @@ -294,30 +285,7 @@
> apr_pool_t *parent_pool = apr_pool_parent_get(child_pool);
>
> if (parent_pool) {
> - /* ideally this should be done by mp_xs_APR__Pool_2obj. Though
> - * since most of the time we don't use custom pools, we don't
> - * want the overhead of reading and writing pool's userdata in
> - * the general case. therefore we do it here and in
> - * mpxs_apr_pool_create. Though if there are any other
> - * functions, that return perl objects whose guts include a
> - * reference to a custom pool, they must do the ref-counting
> - * as well.
> - */
> - mpxs_pool_account_t *data;
> - apr_pool_userdata_get((void **)&data, MP_APR_POOL_NEW, parent_pool);
> - if (data && data->sv) {
> - MP_POOL_TRACE(MP_FUNC,
> - "parent pool (0x%lx) is a custom pool, sv 0x%lx",
> - (unsigned long)parent_pool,
> - (unsigned long)data->sv);
> -
> - return newRV_inc(data->sv);
> - }
> - else {
> - MP_POOL_TRACE(MP_FUNC, "parent pool (0x%lx) is a core pool",
> - (unsigned long)parent_pool);
> - return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> - }
> + return SvREFCNT_inc(mp_xs_APR__Pool_2obj(parent_pool));
> }
> else {
> MP_POOL_TRACE(MP_FUNC, "pool (0x%lx) has no parents",
> @@ -335,11 +303,18 @@
> apr_pool_t *p;
> SV *sv = SvRV(obj);
>
> + p = mpxs_sv_object_deref(obj, apr_pool_t);
> +
> + if (mg_find(sv, PERL_MAGIC_ext))
> + apr_pool_destroy(p);
> +
> +#if 0
> +
> /* MP_POOL_TRACE(MP_FUNC, "DESTROY 0x%lx-0x%lx", */
> /* (unsigned long)obj,(unsigned long)sv); */
> /* do_sv_dump(0, Perl_debug_log, obj, 0, 4, FALSE, 0); */
>
> - p = mpxs_sv_object_deref(obj, apr_pool_t);
> +
> if (!p) {
> /* non-custom pool */
> MP_POOL_TRACE(MP_FUNC, "skip apr_pool_destroy: not a custom pool");
> @@ -367,5 +342,7 @@
> SvREFCNT(sv));
> }
> }
> +
> +#endif
> }
>
>
>
>
>
>
> ------------------------------------------------------------------------
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
[PATCH] Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
>
>>>Any ideas about how to fix the segfaults at startup time too?
>>
>>I'll try to reproduce it now.
>
>
> I'm able to get these segfults now, but not reliably.
> My first impression is that some APR::Pool SV object
> created during post-config, and points at the ptemp pool.
> The problem seems to be that the SV has a slightly longer
> lifespan than the pool does, so when DESTROY is called
> on the SV, the (ptemp) pool pointer it has is bogus.
>
> I think APR__Pool.h would be better insulated from
> this sort of bug if it stored the SV's
>
> "can call apr_pool_destroy"
>
> status in the SV's magic table, instead of the
> pool's userdata.
Please take a look at the long comments in APR__Pool.h, as I doubt that
will work. Any parental pool destroy will call sub-pool destroys, pulling
the rug from under the SVs that refer to sub-pools. Unless you mean
something else. On the other hand Gozer was planning on doing yet another
rewritting of the pool handling, so may be he can chime in and explain
what's on his mind.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
>
>
> Joe Orton wrote:
>
>> On Tue, Sep 28, 2004 at 06:55:32PM -0400, Stas Bekman wrote:
>>
>>> Joe Orton wrote:
>>>
>>>> Index: modperl_filter.c
>>>> ===================================================================
>>>> RCS file:
>>>> /home/cvspublic/modperl-2.0/src/modules/perl/modperl_filter.c,v
>>>> retrieving revision 1.98
>>>> diff -u -r1.98 modperl_filter.c
>>>> --- modperl_filter.c 9 Sep 2004 15:17:20 -0000 1.98
>>>> +++ modperl_filter.c 28 Sep 2004 21:35:28 -0000
>>>> @@ -318,6 +318,8 @@
>>>> }
>>>> filter = (modperl_filter_t *)apr_pcalloc(temp_pool,
>>>> sizeof(*filter));
>>>> + apr_pool_tag(temp_pool, "mod_perl temp filter");
>>>> +
>>>
>>>
>>> but this one happens at run time, what's the overhead of the pool
>>> tagging operation? should we do that only for the maintainer version?
>>
>>
>>
>> The overhead is a function call and a single pointer assignment, i.e.
>> compared to the apr_pcalloc above, pretty much lost in the noise, so
>> it's OK to do it for the non-maintainer build too.
>
>
> It might have low overhead, but if it is only usefull for a maintainer
> kinda person, I think pool tagging like this should be kept for
> maintainer-mode
> builds only.
I agree with Philippe. One needs to enable debug anyway to get any useful
information, and we can always drop that ifdef if there will be a need
later. So I have committed your patch for the filter code with an ifdef:
+#ifdef MP_DEBUG
+ apr_pool_tag(temp_pool, "mod_perl temp filter");
+#endif
and the other one as is.
Thank you, Joe.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Joe Orton wrote:
> On Tue, Sep 28, 2004 at 06:55:32PM -0400, Stas Bekman wrote:
>
>>Joe Orton wrote:
>>
>>>Index: modperl_filter.c
>>>===================================================================
>>>RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_filter.c,v
>>>retrieving revision 1.98
>>>diff -u -r1.98 modperl_filter.c
>>>--- modperl_filter.c 9 Sep 2004 15:17:20 -0000 1.98
>>>+++ modperl_filter.c 28 Sep 2004 21:35:28 -0000
>>>@@ -318,6 +318,8 @@
>>> }
>>> filter = (modperl_filter_t *)apr_pcalloc(temp_pool, sizeof(*filter));
>>>
>>>+ apr_pool_tag(temp_pool, "mod_perl temp filter");
>>>+
>>
>>but this one happens at run time, what's the overhead of the pool tagging
>>operation? should we do that only for the maintainer version?
>
>
> The overhead is a function call and a single pointer assignment, i.e.
> compared to the apr_pcalloc above, pretty much lost in the noise, so
> it's OK to do it for the non-maintainer build too.
It might have low overhead, but if it is only usefull for a maintainer
kinda person, I think pool tagging like this should be kept for maintainer-mode
builds only.
> joe
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
> For additional commands, e-mail: dev-help@perl.apache.org
>
>
--
--------------------------------------------------------------------------------
Philippe M. Chiasson m/gozer\@(apache|cpan|ectoplasm)\.org/ GPG KeyID : 88C3A5A5
http://gozer.ectoplasm.org/ F9BF E0C2 480E 7680 1AE5 3631 CB32 A107 88C3A5A5
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Sep 28, 2004 at 06:55:32PM -0400, Stas Bekman wrote:
> Joe Orton wrote:
> >Index: modperl_filter.c
> >===================================================================
> >RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_filter.c,v
> >retrieving revision 1.98
> >diff -u -r1.98 modperl_filter.c
> >--- modperl_filter.c 9 Sep 2004 15:17:20 -0000 1.98
> >+++ modperl_filter.c 28 Sep 2004 21:35:28 -0000
> >@@ -318,6 +318,8 @@
> > }
> > filter = (modperl_filter_t *)apr_pcalloc(temp_pool, sizeof(*filter));
> >
> >+ apr_pool_tag(temp_pool, "mod_perl temp filter");
> >+
>
> but this one happens at run time, what's the overhead of the pool tagging
> operation? should we do that only for the maintainer version?
The overhead is a function call and a single pointer assignment, i.e.
compared to the apr_pcalloc above, pretty much lost in the noise, so
it's OK to do it for the non-maintainer build too.
joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> On Tue, Sep 28, 2004 at 12:32:36PM -0400, Joe Schaefer wrote:
>
>>Stas Bekman <st...@stason.org> writes:
>>
>>[...]
>>
>>
>>>>Any ideas about how to fix the segfaults at startup time too?
>>>
>>>I'll try to reproduce it now.
>>
>>I'm able to get these segfults now, but not reliably.
>
>
> BTW it was useful to have the pools tagged when looking through the core
> dump, I noticed that mod_perl doesn't do this for all pools.
>
> Index: mod_perl.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
> retrieving revision 1.222
> diff -u -r1.222 mod_perl.c
> --- mod_perl.c 21 Sep 2004 21:35:30 -0000 1.222
> +++ mod_perl.c 28 Sep 2004 21:35:27 -0000
> @@ -539,6 +539,7 @@
> MP_init_status = 1; /* now starting */
>
> apr_pool_create(&server_pool, pconf);
> + apr_pool_tag(server_pool, "mod_perl server pool");
>
> modperl_sys_init();
> apr_pool_cleanup_register(pconf, NULL,
that's fine
> Index: modperl_filter.c
> ===================================================================
> RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_filter.c,v
> retrieving revision 1.98
> diff -u -r1.98 modperl_filter.c
> --- modperl_filter.c 9 Sep 2004 15:17:20 -0000 1.98
> +++ modperl_filter.c 28 Sep 2004 21:35:28 -0000
> @@ -318,6 +318,8 @@
> }
> filter = (modperl_filter_t *)apr_pcalloc(temp_pool, sizeof(*filter));
>
> + apr_pool_tag(temp_pool, "mod_perl temp filter");
> +
but this one happens at run time, what's the overhead of the pool tagging
operation? should we do that only for the maintainer version?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
On Tue, Sep 28, 2004 at 12:32:36PM -0400, Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
>
> [...]
>
> > > Any ideas about how to fix the segfaults at startup time too?
> >
> > I'll try to reproduce it now.
>
> I'm able to get these segfults now, but not reliably.
BTW it was useful to have the pools tagged when looking through the core
dump, I noticed that mod_perl doesn't do this for all pools.
Index: mod_perl.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/mod_perl.c,v
retrieving revision 1.222
diff -u -r1.222 mod_perl.c
--- mod_perl.c 21 Sep 2004 21:35:30 -0000 1.222
+++ mod_perl.c 28 Sep 2004 21:35:27 -0000
@@ -539,6 +539,7 @@
MP_init_status = 1; /* now starting */
apr_pool_create(&server_pool, pconf);
+ apr_pool_tag(server_pool, "mod_perl server pool");
modperl_sys_init();
apr_pool_cleanup_register(pconf, NULL,
Index: modperl_filter.c
===================================================================
RCS file: /home/cvspublic/modperl-2.0/src/modules/perl/modperl_filter.c,v
retrieving revision 1.98
diff -u -r1.98 modperl_filter.c
--- modperl_filter.c 9 Sep 2004 15:17:20 -0000 1.98
+++ modperl_filter.c 28 Sep 2004 21:35:28 -0000
@@ -318,6 +318,8 @@
}
filter = (modperl_filter_t *)apr_pcalloc(temp_pool, sizeof(*filter));
+ apr_pool_tag(temp_pool, "mod_perl temp filter");
+
filter->temp_pool = temp_pool;
filter->mode = mode;
filter->f = f;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:
[...]
> > Any ideas about how to fix the segfaults at startup time too?
>
> I'll try to reproduce it now.
I'm able to get these segfults now, but not reliably.
My first impression is that some APR::Pool SV object
created during post-config, and points at the ptemp pool.
The problem seems to be that the SV has a slightly longer
lifespan than the pool does, so when DESTROY is called
on the SV, the (ptemp) pool pointer it has is bogus.
I think APR__Pool.h would be better insulated from
this sort of bug if it stored the SV's
"can call apr_pool_destroy"
status in the SV's magic table, instead of the
pool's userdata.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> Let's continue this on dev@perl since it doesn't seem like there's an
> httpd bug currently.
>
> On Mon, Sep 27, 2004 at 01:49:29PM -0400, Joe Schaefer wrote:
>
>>Joe Orton <jo...@redhat.com> writes:
>>
>>[...]
>>
>>
>>>when it did start up it would segfault consistently when running the
>>>tests you reference like:
>>>
>>>#0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
>>> at memory/unix/apr_pools.c:1341
>>>1341 while (pool->child)
>
> ...
>
>>Does this mp2 patch help?
>
>
> Yes, that fixed the segfaults triggered by the test cases for me.
joe++,
$subrequest->DESTROY calls ap_destroy_sub_req(r)
the bug was in calling it twice, once directly and once more indirectly.
But the problem is that it didn't affect the current request, but some
other specific request later on, so if it wasn't for Joe, I'd have lost a
way more hair than I did so far. I suppose that double call was corrupting
some memory, but it wasn't visible right away. Is it possible for
ap_destroy_sub_req(r) to somehow protect from this kind of problem? I
suppose that's not quite possible.
> Any ideas about how to fix the segfaults at startup time too?
I'll try to reproduce it now.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> Let's continue this on dev@perl since it doesn't seem like there's an
> httpd bug currently.
>
> On Mon, Sep 27, 2004 at 01:49:29PM -0400, Joe Schaefer wrote:
>
>>Joe Orton <jo...@redhat.com> writes:
>>
>>[...]
>>
>>
>>>when it did start up it would segfault consistently when running the
>>>tests you reference like:
>>>
>>>#0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
>>> at memory/unix/apr_pools.c:1341
>>>1341 while (pool->child)
>
> ...
>
>>Does this mp2 patch help?
>
>
> Yes, that fixed the segfaults triggered by the test cases for me.
joe++,
$subrequest->DESTROY calls ap_destroy_sub_req(r)
the bug was in calling it twice, once directly and once more indirectly.
But the problem is that it didn't affect the current request, but some
other specific request later on, so if it wasn't for Joe, I'd have lost a
way more hair than I did so far. I suppose that double call was corrupting
some memory, but it wasn't visible right away. Is it possible for
ap_destroy_sub_req(r) to somehow protect from this kind of problem? I
suppose that's not quite possible.
> Any ideas about how to fix the segfaults at startup time too?
I'll try to reproduce it now.
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
Let's continue this on dev@perl since it doesn't seem like there's an
httpd bug currently.
On Mon, Sep 27, 2004 at 01:49:29PM -0400, Joe Schaefer wrote:
> Joe Orton <jo...@redhat.com> writes:
>
> [...]
>
> > when it did start up it would segfault consistently when running the
> > tests you reference like:
> >
> > #0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
> > at memory/unix/apr_pools.c:1341
> > 1341 while (pool->child)
...
> Does this mp2 patch help?
Yes, that fixed the segfaults triggered by the test cases for me. Any
ideas about how to fix the segfaults at startup time too?
joe
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
Let's continue this on dev@perl since it doesn't seem like there's an
httpd bug currently.
On Mon, Sep 27, 2004 at 01:49:29PM -0400, Joe Schaefer wrote:
> Joe Orton <jo...@redhat.com> writes:
>
> [...]
>
> > when it did start up it would segfault consistently when running the
> > tests you reference like:
> >
> > #0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
> > at memory/unix/apr_pools.c:1341
> > 1341 while (pool->child)
...
> Does this mp2 patch help?
Yes, that fixed the segfaults triggered by the test cases for me. Any
ideas about how to fix the segfaults at startup time too?
joe
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Orton <jo...@redhat.com> writes:
[...]
> Yes, that fixed the segfaults triggered by the test cases for me. Any
> ideas about how to fix the segfaults at startup time too?
Reposting the backtrace: http://www.apache.org/~jorton/startup.perl
I'm not able to reproduce the startup segfaults yet [1].
Here's my config.nice for httpd:
#! /bin/sh
#
# Created by configure
CPPFLAGS="-DAPR_BUCKET_DEBUG"; export CPPFLAGS
"./configure" \
"--with-mpm=worker" \
"--enable-so" \
"--enable-mods-shared=all" \
"--enable-ssl=shared" \
"--enable-proxy=shared" \
"--enable-maintainer-mode" \
"--enable-pool-debug" \
"$@"
[1] various mp2 tests are hanging with today's httpd/apr HEAD. I've
tested with cvs co -D2004-09-21 modperl-2.0 and see the same behavior.
--
Joe Schaefer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Schaefer <jo...@sunstarsys.com>.
Joe Orton <jo...@redhat.com> writes:
[...]
> when it did start up it would segfault consistently when running the
> tests you reference like:
>
> #0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
> at memory/unix/apr_pools.c:1341
> 1341 while (pool->child)
> #0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
> at memory/unix/apr_pools.c:1341
> #1 0x0000002a95d35db4 in apr_pool_destroy_debug (pool=0x441,
> file_line=0x2a95d3fc19 "undefined") at memory/unix/apr_pools.c:1426
> #2 0x0000002a95d35ce6 in pool_clear_debug (pool=0x1a223c0,
> file_line=0x2a95d3fc19 "undefined")
> at memory/unix/apr_pools.c:1342
> #3 0x0000002a95d35db4 in apr_pool_destroy_debug (pool=0x1a223c0,
> file_line=0x2a95d3fc19 "undefined") at memory/unix/apr_pools.c:1426
> #4 0x0000002a95d369b0 in apr_pool_destroy (pool=0x441) at
> memory/unix/apr_pools.c:2206
> #5 0x0000002a9e1fb8ff in XS_Apache__SubRequest_DESTROY
> (my_perl=0x2525420, cv=0x1844ff0)
> at SubRequest.xs:36
Does this mp2 patch help?
Index: t/response/TestAPI/sub_request.pm
===================================================================
RCS file: /home/cvspublic/modperl-2.0/t/response/TestAPI/sub_request.pm,v
retrieving revision 1.1
diff -u -r1.1 sub_request.pm
--- t/response/TestAPI/sub_request.pm 14 Aug 2004 05:10:12 -0000 1.1
+++ t/response/TestAPI/sub_request.pm 27 Sep 2004 17:48:01 -0000
@@ -34,7 +34,7 @@
# test an explictit DESTROY (which happens automatically on the
# scope exit)
- $subr->DESTROY;
+ undef $subr;
ok 1;
Apache::OK;
--
Joe Schaefer
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
On Mon, Sep 27, 2004 at 08:28:49AM -0400, Stas Bekman wrote:
> Yup, I can reliably (90% of runs) reproduce that with prefork mpm by
> running this sequence:
> t/TEST t/filter/out_str_reverse.t t/apr/table.t t/filter/out_str_req_mix.t
> t/api/sub_request.t t/api/aplog.t t/modperl/merge.t
Do you know when this started failing? It appears to be unrelated to my
ap_save_brigade change, since it fails just the same with that reverted.
It looks like memory corruption: I tried those tests against a server
built using --enable-pool-debug but then mod_perl will rarely start up,
segfaulting in:
#0 0x0000002a95d2f2c2 in find_entry (ht=0x8, key=0x2a9eead7f6, klen=-1, val=0x0)
at tables/apr_hash.c:253
253 hash = ht->hash_func(key, &klen);
#0 0x0000002a95d2f2c2 in find_entry (ht=0x8, key=0x2a9eead7f6, klen=-1, val=0x0)
at tables/apr_hash.c:253
#1 0x0000002a95d2f45b in apr_hash_get (ht=0x2a9eead7f6, key=0x7fbfff8ac8, klen=-1)
at tables/apr_hash.c:319
#2 0x0000002a95d36518 in apr_pool_userdata_get (data=0x7fbfff8b30, key=0x2a9eead7f6 "APR::Pool::new", pool=0x573d40)
at memory/unix/apr_pools.c:1864
#3 0x0000002a9eeac786 in mpxs_apr_pool_DESTROY (my_perl=0x2a9eead7f6, obj=0x7fbfff8ac8)
at APR__Pool.h:352
#4 0x0000002a9eead34c in XS_APR__Pool_DESTROY (my_perl=0x7e2b30, cv=0x7fbfff8ac8)
at Pool.xs:119
#5 0x0000002a966c2358 in Perl_pp_entersub (my_perl=0x7e2b30)
at pp_hot.c:2781
#6 0x0000002a96668154 in S_call_body (my_perl=0x7e2b30, myop=0x7fbfff8ac8, is_eval=-1)
at perl.c:2066
etc at http://www.apache.org/~jorton/startup.perl
when it did start up it would segfault consistently when running the
tests you reference like:
#0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
at memory/unix/apr_pools.c:1341
1341 while (pool->child)
#0 pool_clear_debug (pool=0x441, file_line=0x2a95d3fc19 "undefined")
at memory/unix/apr_pools.c:1341
#1 0x0000002a95d35db4 in apr_pool_destroy_debug (pool=0x441,
file_line=0x2a95d3fc19 "undefined") at memory/unix/apr_pools.c:1426
#2 0x0000002a95d35ce6 in pool_clear_debug (pool=0x1a223c0,
file_line=0x2a95d3fc19 "undefined")
at memory/unix/apr_pools.c:1342
#3 0x0000002a95d35db4 in apr_pool_destroy_debug (pool=0x1a223c0,
file_line=0x2a95d3fc19 "undefined") at memory/unix/apr_pools.c:1426
#4 0x0000002a95d369b0 in apr_pool_destroy (pool=0x441) at
memory/unix/apr_pools.c:2206
#5 0x0000002a9e1fb8ff in XS_Apache__SubRequest_DESTROY
(my_perl=0x2525420, cv=0x1844ff0)
at SubRequest.xs:36
etc at http://www.apache.org/~jorton/pool.debug
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Joe Orton wrote:
> On Sun, Sep 26, 2004 at 07:45:24PM -0400, Stas Bekman wrote:
>
>>I get the following segfault with the latest cvs 2.0 branch:
>>
>>#0 0x080eaed2 in ap_save_brigade (f=0x9541448, saveto=0x9541488,
>> b=0xbfffebd4, p=0x9550ba8) at util_filter.c:539
>>539 APR_BRIGADE_CONCAT(*saveto, *b);
>
>
> Is this triggered in the mod_perl test suite or something?
Yup, I can reliably (90% of runs) reproduce that with prefork mpm by
running this sequence:
t/TEST t/filter/out_str_reverse.t t/apr/table.t t/filter/out_str_req_mix.t
t/api/sub_request.t t/api/aplog.t t/modperl/merge.t
>>(gdb) p (**saveto)->list
>>$4 = {next = 0x206e7553, prev = 0x20706553}
>>(gdb) dump_brigade b
>
>
> You have to "dump_brigade *b" in ap_save_brigade since it's a
> pointer-to-a-pointer-to-a-brigade structure, can you post that output?
(gdb) dump_brigade *b
dump of brigade 0x9550be0
| type (address) | length | data addr | contents
| rc
--------------------------------------------------------------------------------
0 | Cannot access memory at address 0x20726f6e
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: segfault ap_save_brigage in the latest 2.0
Posted by Joe Orton <jo...@redhat.com>.
On Sun, Sep 26, 2004 at 07:45:24PM -0400, Stas Bekman wrote:
> I get the following segfault with the latest cvs 2.0 branch:
>
> #0 0x080eaed2 in ap_save_brigade (f=0x9541448, saveto=0x9541488,
> b=0xbfffebd4, p=0x9550ba8) at util_filter.c:539
> 539 APR_BRIGADE_CONCAT(*saveto, *b);
Is this triggered in the mod_perl test suite or something?
> (gdb) p (**saveto)->list
> $4 = {next = 0x206e7553, prev = 0x20706553}
> (gdb) dump_brigade b
You have to "dump_brigade *b" in ap_save_brigade since it's a
pointer-to-a-pointer-to-a-brigade structure, can you post that output?
joe
Re: segfault ap_save_brigage in the latest 2.0
Posted by Cliff Woolley <jw...@virginia.edu>.
On Mon, 27 Sep 2004, Stas Bekman wrote:
> Thanks Cliff, and how do I enable that?
Add -DAPR_BUCKET_DEBUG to the CFLAGS passed to configure. See page 17 of
http://www.cs.virginia.edu/~jcw5q/talks/apache/apache2moddebugging.pdf for
an example.
--Cliff
Re: segfault ap_save_brigage in the latest 2.0
Posted by Stas Bekman <st...@stason.org>.
Cliff Woolley wrote:
[...]
> Looking at the ->prev pointer of b, it seems that the pointer there is way
> out of range compared to the other pointer values you gave. And brigade b
> is obviously corrupt, so... yeah. So the question is, at what point did
> brigade b get corrupted. The easiest way to find that out is to enable
> brigade debugging -- that way, any time you do any manipulations to the
> brigade that could potentially corrupt it, it will sanity-check the ring
> pointers in both directions for you. That way it will assert when the
> corruption happens rather that some arbitrary time in the future, which is
> what you're seeing here.
Thanks Cliff, and how do I enable that?
--
__________________________________________________________________
Stas Bekman JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/ mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org http://ticketmaster.com
Re: segfault ap_save_brigage in the latest 2.0
Posted by Cliff Woolley <jw...@virginia.edu>.
Ouch. This one kinda sucks. Well done on the debugging dumps though...
you grabbed almost all of the things I was going to ask for. :)
Let's see if I can make any sense of this.
> myfunc does:
>
> bucket = apr_bucket_transient_create(buf, len, ba);
> bb = apr_brigade_create(wb->pool, ba);
> APR_BRIGADE_INSERT_TAIL(bb, bucket);
> ap_pass_brigade(f, bb);
Seems reasonable.
> (gdb) source /home/stas/apache.org/httpd-2.0/.gdbinit
> (gdb) dump_bucket bucket
> bucket=HEAP (0x09544f98) length=1455 data=0x09545178
> contents=[1..10~# Running u...] rc=1
> (gdb) dump_brigade bb
> dump of brigade 0x9554e78
> | type (address) | length | data addr | contents
> | rc
> --------------------------------------------------------------------------------
> brigade is empty
> end of brigade
All this seems reasonable too. Note that the transient bucket you created
has morphed into a heap bucket (as it ought to when you run save_brigade,
which calls setaside on all of the buckets -- and transient setaside does
a alloc-morph-memcpy onto the heap).
> (gdb) dump_brigade b
> dump of brigade 0xbfffebd4
> | type (address) | length | data addr | contents
> | rc
> --------------------------------------------------------------------------------
> 0 | § (0xbfffebf8) | 156585592 | 0x405cbc18 | [**unknown**] | n/a
> 1 | ÃÃl
> (0xbfffec48) | 156585592 | 0x404edd4e | [**unknown**] | n/a
> 2 | ÃÃl
> (0xbfffec78) | 156585592 | 0x00000004 | [**unknown**] | n/a
> 3 | ¨ (0xbfffecc8) | 156585592 | 0x080ead44 | [**unknown**] | n/a
> 4 | ¨ (0xbfffecf8) | 156585592 | 0xbfffed9c | [**unknown**] | n/a
> 5 |#
> (0xbfffed68) | 156585592 | 0x00000000 | [**unknown**] | n/a
> 6 |#
> (0xbfffed98) | 156585592 | 0x4044d9fd | [**unknown**] | n/a
> 7 | Cannot access memory at address 0x0
Yowzah. Um yeah that's obviously bad. :)
Looking at the ->prev pointer of b, it seems that the pointer there is way
out of range compared to the other pointer values you gave. And brigade b
is obviously corrupt, so... yeah. So the question is, at what point did
brigade b get corrupted. The easiest way to find that out is to enable
brigade debugging -- that way, any time you do any manipulations to the
brigade that could potentially corrupt it, it will sanity-check the ring
pointers in both directions for you. That way it will assert when the
corruption happens rather that some arbitrary time in the future, which is
what you're seeing here.
Hope this helps,
Cliff