You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@perl.apache.org by Stas Bekman <st...@stason.org> on 2004/11/25 17:38:45 UTC

[mp2] pools that go out of scope aren't a problem anymore?

Hmm, I can't reproduce this problem anymore:

* pools that go out of scope:

   perl -MApache2 -MAPR::Pool -MAPR::PerlIO -le ';
   open my $fh, "<:APR", "/tmp/xxx", APR::Pool->new; print <$fh>'
   APR::PerlIO::read: (9) Bad file descriptor at -e li

   first of all, try to never allow passing temp pools, e.g in the
   above example, we can check that the pool object doesn't have the
   TEMP flag on.

   but that doesn't really solve the problem. So we aren't sure how to
   deal with that.

tried also:

---------
use blib;
use Apache2;

use APR::Pool ();
use APR::Table ();

$table = APR::Table::make(APR::Pool->new, $nelts);

$table->set(a => 5);

print $table->get('a');
-------

and it works as well.

So does that meant that the latest reincarnation of APR::Pool 
implementation has this problem fixed? Or are there any other situations 
where this needs to be fixed?

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> I've rebuit with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", but 
> also apr1/httpd2.1
> 
>>>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>>>   $t= APR::Table::make(APR::Pool->new, 10);   $t->set($_=>$_), print 
>>> "Set $_" for 1..20'
>>>
>>>   Segmentation fault
> 
> 
> Now I get the segfault

I get it also with apr-0.9/httpd-2.0.

I'm back to work on this temp pool object issue. I've found what I think 
is another problem.

APR::BucketAlloc
apr_bucket_alloc_create(p)
     APR::Pool p

So if we link pool to alloc, everything is cool, but when this alloc is 
used to create something and then goes out of scope as in:

   {
      my $ba = APR::Bucket::alloc_create(APR::Pool->new);
      $b = APR::Bucket::eos_create($ba);
   }

now $b guts are wiped, since $ba and the temp pool are gone. Am I right?

In which case it's not enough to make pool's destroy on the other objects 
that use it, but it goes further down the chain, in this example the 
bucket needs to link the allocator so that doesn't get nuked before the 
bucket is done with its scope?

There are probably other chain dependency issues.

I think there is one more problem with preventing from the temp object 
pools being destroyed this is related to the problem previously discovered 
with modperl_bucket.c:

     /* PADTMP SVs belong to perl and can't be stored away, since perl
      * is going to reuse them, so we have no choice but to copy the
      * data away, before storing sv */
     if (SvPADTMP(sv)) {
         STRLEN len;
         char *pv = SvPV(sv, len);
         svbucket->sv = newSVpvn(pv, len);
     }

so for example I think this code:

   sub make_table {
      return APR::Table::make(APR::Pool->new, 10);
   }
   push @tables, make_table() for 1..2;
   for my $table (@tables) {
       $table->set($_ => $_) for 1..20;
   }
   for my $table (@tables) {
       print $table->get($_) for 1..20;
   }

has a problem, since the temp pool in the sub is PADTMP SV, which means 
that perl will want to reuse the same SV everytime the sub is entered. so 
what happens to the pool which can't be destroyed, but perl will overwrite 
it with a new data. I didn't actually run the test yet.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Philippe M. Chiasson wrote:
> [...]
> 
>> For instance, for each request, we create a modperl_request_pool() and 
>> $r->pool returns that,
>> not the underlying $r->pool. We just register a cleanup handler with 
>> $r->pool to destroy our
>> request pool, but use a refcount technique to figure out if the pool 
>> can be safely freed. If not,
>> we either die verbosely, or just warn about it
>>
>>  "Warning: prolonging the lifetime of request pool at foo/bar.pl line 
>> 232"
>>
>> Certainly possible to implement, but I am just not sure we can justify 
>> this added magic/complexity
>> simply for the sake of catching improper uses of the API.
> 
> Too complicated. It's not worth the very small probability of people 
> writing this kind of really bad code.

As long as we are ready to help/support the few people that _do_ write that
kind of bad code, I am happy not having to implement something like that.

>>> Unfortunately it doesn't seem like this dependency code can be easily 
> [...]

-- 
--------------------------------------------------------------------------------
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

Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:
[...]
> For instance, for each request, we create a modperl_request_pool() and 
> $r->pool returns that,
> not the underlying $r->pool. We just register a cleanup handler with 
> $r->pool to destroy our
> request pool, but use a refcount technique to figure out if the pool can 
> be safely freed. If not,
> we either die verbosely, or just warn about it
> 
>  "Warning: prolonging the lifetime of request pool at foo/bar.pl line 232"
> 
> Certainly possible to implement, but I am just not sure we can justify 
> this added magic/complexity
> simply for the sake of catching improper uses of the API.

Too complicated. It's not worth the very small probability of people 
writing this kind of really bad code.

>> Unfortunately it doesn't seem like this dependency code can be easily 
>> integrated with the automatic type conversion, because the code 
>> accepts a single argument in xs/modperl_xs_sv_convert.h.
> 
> 
> I am not sure about this one, but couldn't we use our fancy XS 
> generation framerowk
> to magically detect methods with a pool argument and facilitate this 
> somewhat ?
> 
>> At the moment we will need to rewrite all methods that accept the pool 
>> object, like I did for APR::Table::make above. for example inside 
>> APR::Table there are also copy() and overlay() that need the same 
>> solution.
> 
> 
> And that sounds like a terrible idea to me, unless we can make it really 
> tiny,
> like a single macro or sth. Still, I think it would be much more 
> error-prone than
> figuring this out somewhat automatically. If the typemap stuff isn't 
> sufficient,
> why can't we just implement our own sort of meta-typemap from all the 
> neat data
> we got in xs/tables ?

I'll give it a try.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Philippe M. Chiasson wrote:

>>> This is a problem with persistence, not "ownership", and affects the
>>> entirety of mp2's object system.  In other words, there's no reason 
>>> to single out pools in your example, the bug could just as easily been
>>>
>>> my $r;
>>>
>>>    sub handler {
>>>        $r ||= shift;
>>>        my $pool = $r->pool; #bam!
>>>    }
[...]
> I agree. I think it's important to make sure that whatever perl code our 
> users
> end up writing, that they are not able to segfault the server. 
> Preventing/detecting
> this particular kind of problem will be tricky though.

The penalty of doing that will be too big for the normal uses. So 
hopefully we won't have to deal with that too much. And in the worst case, 
we will try to resolve the issue if we will have it.


-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.

Stas Bekman wrote:
> Joe Schaefer wrote:
> 
>> "Philippe M. Chiasson" <go...@ectoplasm.org> writes:
>>
>>
>>> my $pool;
>>> sub handler {
>>>  my $r = shift;
>>>  $pool ||= $r->pool; #XXX: Yes, bad bad bad, but still...
>>>  APR::Table::make($pool, 10); #bam!
>>> }
>>>
>>> The problem in this case is that apache destroyed the request pool at
>>> the end of the first request. And we now have a valid $pool that now
>>> points to a freed pool. It's an incorrect usage of the API, I know,
>>> but the resulting segv isn't nice... 
>>
>>
>>
>> This is a problem with persistence, not "ownership", and affects the
>> entirety of mp2's object system.  In other words, there's no reason to 
>> single out pools in your example, the bug could just as easily been
>>
>> my $r;
>>
>>    sub handler {
>>        $r ||= shift;
>>        my $pool = $r->pool; #bam!
>>    }
>>
>> "Fixing" this problem is certainly a challenge, but IMO not
>> worth worrying about for 2.0.
> 
> Agreed.

I agree as well.

> That's said this issue is not critical for RC1, since they user API 
> won't change, we can work on it in parallel, but certainly will finish 
> it before the gold release.

I agree. I think it's important to make sure that whatever perl code our users
end up writing, that they are not able to segfault the server. Preventing/detecting
this particular kind of problem will be tricky though.
 
--------------------------------------------------------------------------------
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

Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> "Philippe M. Chiasson" <go...@ectoplasm.org> writes:
> 
> 
>>my $pool;
>>sub handler {
>>  my $r = shift;
>>  $pool ||= $r->pool; #XXX: Yes, bad bad bad, but still...
>>  APR::Table::make($pool, 10); #bam!
>>}
>>
>>The problem in this case is that apache destroyed the request pool at
>>the end of the first request. And we now have a valid $pool that now
>>points to a freed pool. It's an incorrect usage of the API, I know,
>>but the resulting segv isn't nice... 
> 
> 
> This is a problem with persistence, not "ownership", and affects the
> entirety of mp2's object system.  In other words, there's no reason to 
> single out pools in your example, the bug could just as easily been
> 
> my $r;
> 
>    sub handler {
>        $r ||= shift;
>        my $pool = $r->pool; #bam!
>    }
> 
> "Fixing" this problem is certainly a challenge, but IMO not
> worth worrying about for 2.0.

Agreed.

That's said this issue is not critical for RC1, since they user API won't 
change, we can work on it in parallel, but certainly will finish it before 
the gold release.


-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
"Philippe M. Chiasson" <go...@ectoplasm.org> writes:

> my $pool;
> sub handler {
>   my $r = shift;
>   $pool ||= $r->pool; #XXX: Yes, bad bad bad, but still...
>   APR::Table::make($pool, 10); #bam!
> }
>
> The problem in this case is that apache destroyed the request pool at
> the end of the first request. And we now have a valid $pool that now
> points to a freed pool. It's an incorrect usage of the API, I know,
> but the resulting segv isn't nice... 

This is a problem with persistence, not "ownership", and affects the
entirety of mp2's object system.  In other words, there's no reason to 
single out pools in your example, the bug could just as easily been

my $r;

   sub handler {
       $r ||= shift;
       my $pool = $r->pool; #bam!
   }

"Fixing" this problem is certainly a challenge, but IMO not
worth worrying about for 2.0.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by "Philippe M. Chiasson" <go...@ectoplasm.org>.
Stas Bekman wrote:
> Stas Bekman wrote:
> 
>> I've rebuit with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", 
>> but also apr1/httpd2.1
>>
>>>>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>>>>   $t= APR::Table::make(APR::Pool->new, 10);   $t->set($_=>$_), print 
>>>> "Set $_" for 1..20'
>>>>
>>>>   Segmentation fault
>>
>> Now I get the segfault

Yes, now I remember that the segv are usually happenning whith --enable-pool-debug
 
> So trying to go the way Joe has chosen for apreq, I've replaced the 
> plain apr_table_make autogenerated wrapper with explicit:
> 
> static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
> {
>     apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>     apr_table_t *t = apr_table_make(p, nelts);
>     SV *t_sv = mp_xs_APR__Table_2obj(t);
>     sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>     return t_sv;
> }
> 
> and segfault is gone. here we create a dependency p_sv <- t_sv, so only 
> when t_sv goes out of scope the pool object will be destroyed, and this 
> code is now safe:
> 
>   APR::Table::make(APR::Pool->new, 10);

Well, there remains the _other_ pool issue I caught the other day. This
_bad_ code example should illustrate.

my $pool;
sub handler {
  my $r = shift;
  $pool ||= $r->pool; #XXX: Yes, bad bad bad, but still...
  APR::Table::make($pool, 10); #bam!
}

The problem in this case is that apache destroyed the request pool at the end of the first
request. And we now have a valid $pool that now points to a freed pool. It's an incorrect
usage of the API, I know, but the resulting segv isn't nice...

I did start thinking about a solution to this kind of problem, but I am not sure it's a very
good suggestion. I was thinking along the way of _never_ handing off to Perl land SV wrappers
to external pool objects (i.e. httpd's), but instead, manage our own memory pools in parallel
and allow us to detect/trap such _bad_ usages.

For instance, for each request, we create a modperl_request_pool() and $r->pool returns that,
not the underlying $r->pool. We just register a cleanup handler with $r->pool to destroy our
request pool, but use a refcount technique to figure out if the pool can be safely freed. If not,
we either die verbosely, or just warn about it

  "Warning: prolonging the lifetime of request pool at foo/bar.pl line 232"

Certainly possible to implement, but I am just not sure we can justify this added magic/complexity
simply for the sake of catching improper uses of the API.

> Unfortunately it doesn't seem like this dependency code can be easily 
> integrated with the automatic type conversion, because the code accepts 
> a single argument in xs/modperl_xs_sv_convert.h.

I am not sure about this one, but couldn't we use our fancy XS generation framerowk
to magically detect methods with a pool argument and facilitate this somewhat ?

> At the moment we will need to rewrite all methods that accept the pool 
> object, like I did for APR::Table::make above. for example inside 
> APR::Table there are also copy() and overlay() that need the same solution.

And that sounds like a terrible idea to me, unless we can make it really tiny,
like a single macro or sth. Still, I think it would be much more error-prone than
figuring this out somewhat automatically. If the typemap stuff isn't sufficient,
why can't we just implement our own sort of meta-typemap from all the neat data
we got in xs/tables ?

> Any suggestions?
>
> I'm thinking whether we could use the same solution for PV return values 
> that use the pool object. For example with server_root_relative(), see 
> the end of notes at:
> http://perl.apache.org/docs/2.0/api/Apache/ServerUtil.html#C_server_root_relative_ 
> 
> which documents the danger of having the SvPV having a hole in body if 
> the pool went out of scope. I guess we can attach the same magical 
> dependency there too.

Yes, that the reverse problem of my example. And the same solution would possibly
apply.

--------------------------------------------------------------------------------
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

Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
>> +static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
>> +{
>> +    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>> +    apr_table_t *t = apr_table_make(p, nelts);
>> +    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
>> +    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>> +    return t_sv;
>> +}
> 
> 
> And that just happened to work, since it wasn't 5.8.x+
> 
> sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
> 
> can't be used since it's already used by:
> 
> MP_INLINE SV *modperl_hash_tie(pTHX_
> [...]
> 
>     /* Prefetch magic requires perl 5.8 */
> #if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))
> 
>     sv_magic(hv, NULL, PERL_MAGIC_ext, Nullch, -1);
>     SvMAGIC(hv)->mg_virtual = (MGVTBL *)&modperl_table_magic_prefetch;
>     SvMAGIC(hv)->mg_flags |= MGf_COPY;
> 
> #endif /* End of prefetch magic */
> 
>     sv_magic(hv, rsv, PERL_MAGIC_tied, Nullch, 0);
> 
> so it happened to worked before I guess because I was testing with 5.6.x,
> 
> with 5.8.x, if I dump the table object it has only one _ext magic.
> 
> so we need to use some other magic to create this dependency.

In case someone wants to play with it here is the latest patch including 
the segfaulting tests.

Index: xs/maps/apr_functions.map
===================================================================
--- xs/maps/apr_functions.map	(revision 122696)
+++ xs/maps/apr_functions.map	(working copy)
@@ -245,7 +245,8 @@
  MODULE=APR::Table
   apr_table_clear
   apr_table_copy    | | t, p
- apr_table_make
+~apr_table_make
+ mpxs_APR__Table_make
   apr_table_overlap
   apr_table_overlay | | base, overlay, p
   apr_table_compress
Index: xs/APR/Table/APR__Table.h
===================================================================
--- xs/APR/Table/APR__Table.h	(revision 122696)
+++ xs/APR/Table/APR__Table.h	(working copy)
@@ -17,6 +17,18 @@
  #define mpxs_APR__Table_DELETE  apr_table_unset
  #define mpxs_APR__Table_CLEAR   apr_table_clear

+static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
+{
+    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
+    apr_table_t *t = apr_table_make(p, nelts);
+    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
+    sv_dump(SvRV(p_sv));
+    /* XXX: this seems to be ignored by perl 5.8.x+, since
+     * modperl_hash_tie already attached another _ext magic */
+    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
+    return t_sv;
+}
+
  typedef struct {
      SV *cv;
      apr_hash_t *filter;
Index: xs/tables/current/ModPerl/FunctionTable.pm
===================================================================
--- xs/tables/current/ModPerl/FunctionTable.pm	(revision 122696)
+++ xs/tables/current/ModPerl/FunctionTable.pm	(working copy)
@@ -2,7 +2,7 @@

  # !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
  # ! WARNING: generated by ModPerl::ParseSource/0.01
-# !          Fri Dec 17 21:23:11 2004
+# !          Fri Dec 17 21:24:23 2004
  # !          do NOT edit, any changes will be lost !
  # !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!

@@ -5921,6 +5921,28 @@
      ]
    },
    {
+    'return_type' => 'SV *',
+    'name' => 'mpxs_APR__Table_make',
+    'attr' => [
+      'static',
+      '__inline__'
+    ],
+    'args' => [
+      {
+        'type' => 'PerlInterpreter *',
+        'name' => 'my_perl'
+      },
+      {
+        'type' => 'SV *',
+        'name' => 'p_sv'
+      },
+      {
+        'type' => 'int',
+        'name' => 'nelts'
+      }
+    ]
+  },
+  {
      'return_type' => 'char *',
      'name' => 'mpxs_APR__URI_port',
      'args' => [
Index: t/lib/TestAPRlib/table.pm
===================================================================
--- t/lib/TestAPRlib/table.pm	(revision 122696)
+++ t/lib/TestAPRlib/table.pm	(working copy)
@@ -17,7 +17,7 @@
  our $filter_count;

  sub num_of_tests {
-    my $tests = 50;
+    my $tests = 52;

      # tied hash values() for a table w/ multiple values for the same
      # key
@@ -295,6 +295,30 @@
          ok t_cmp($foo[0], 'one, two, three');
          ok t_cmp($bar[0], 'beer');
      }
+
+
+    # temp pool objects.
+    # testing here that the temp pool object doesn't go out of scope
+    # before the object based on it was freed. the following tests
+    # were previously segfaulting when using apr1/httpd2.1 built w/
+    # --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG",
+    {
+        my $table = APR::Table::make(APR::Pool->new, 10);
+        use Devel::Peek;
+        Dump $table;
+        $table->set($_ => $_) for 1..20;
+        ok t_cmp $table->get(20), 20, "no segfault";
+    }
+    {
+        {
+            my $p = APR::Pool->new;
+            $p->cleanup_register(sub { "whatever" });
+            $table = APR::Table::make($p, 10)
+        };
+        $table->set(a => 5);
+        ok t_cmp $table->get("a"), 5, "no segfault";
+    }
+
  }

  sub my_filter {


-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
> [...]
> 
> 
>>in fact the only reason modperl_hash_tie calls sv_magic is to assign some
>>flags, may be it can be eliminated completely. I haven't looked at the other
>>places where it is used. Why do you think the order matters, if it doesn't
>>really use that magic via mg_find.
> 
> 
> Certainly we don't ever need/want to mg_find() the magic from
> mpxs_APR__Table_make.  I don't know enough about the modperl_hash_tie 
> magic to say it's never needed either, so I definitely trust your 
> judgement about it.

I'll work on that.

BTW, with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", an existing 
test t/apr-ext/threadmutex.t segfault for the same reason under perl 5.6.x 
-- the pool goes out of scope before the mutex is destroyed.


-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:


[...]

> in fact the only reason modperl_hash_tie calls sv_magic is to assign some
> flags, may be it can be eliminated completely. I haven't looked at the other
> places where it is used. Why do you think the order matters, if it doesn't
> really use that magic via mg_find.

Certainly we don't ever need/want to mg_find() the magic from
mpxs_APR__Table_make.  I don't know enough about the modperl_hash_tie 
magic to say it's never needed either, so I definitely trust your 
judgement about it.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
>>>+static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
>>>+{
>>>+    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>>>+    apr_table_t *t = apr_table_make(p, nelts);
>>>+    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
>>>+    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>>>+    return t_sv;
>>>+}
>>
>>And that just happened to work, since it wasn't 5.8.x+
>>
>>sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>>
>>can't be used since it's already used by:
>>
>>MP_INLINE SV *modperl_hash_tie(pTHX_
>>[...]
>>
>>     /* Prefetch magic requires perl 5.8 */
>>#if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))
>>
>>     sv_magic(hv, NULL, PERL_MAGIC_ext, Nullch, -1);
>>     SvMAGIC(hv)->mg_virtual = (MGVTBL *)&modperl_table_magic_prefetch;
>>     SvMAGIC(hv)->mg_flags |= MGf_COPY;
>>
>>#endif /* End of prefetch magic */
>>
>>     sv_magic(hv, rsv, PERL_MAGIC_tied, Nullch, 0);
>>
>>so it happened to worked before I guess because I was testing with 5.6.x,
>>
>>with 5.8.x, if I dump the table object it has only one _ext magic.
>>
>>so we need to use some other magic to create this dependency.
> 
> 
> You probably just need to use sv_magicext with 5.8.x, because
> sv_magic doesn't seem to permit duplicates.  The only issue
> then is ordering: you want the mpxs_APR__Table_make one further 
> down the SvMAGIC chain than the modperl_hash_tie one.

joe++, it works. but man, talking about automating this kind of 
wrappers... here is the partial patch with tweaks suggested from Joe:

in fact the only reason modperl_hash_tie calls sv_magic is to assign some 
flags, may be it can be eliminated completely. I haven't looked at the 
other places where it is used. Why do you think the order matters, if it 
doesn't really use that magic via mg_find.

Index: src/modules/perl/modperl_common_util.c
===================================================================
--- src/modules/perl/modperl_common_util.c	(revision 122696)
+++ src/modules/perl/modperl_common_util.c	(working copy)
@@ -69,7 +69,7 @@
      /* Prefetch magic requires perl 5.8 */
  #if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))

-    sv_magic(hv, NULL, PERL_MAGIC_ext, Nullch, -1);
+    sv_magicext(hv, NULL, PERL_MAGIC_ext, NULL, Nullch, -1);
      SvMAGIC(hv)->mg_virtual = (MGVTBL *)&modperl_table_magic_prefetch;
      SvMAGIC(hv)->mg_flags |= MGf_COPY;

Index: xs/APR/Table/APR__Table.h
===================================================================
--- xs/APR/Table/APR__Table.h	(revision 122696)
+++ xs/APR/Table/APR__Table.h	(working copy)
@@ -17,6 +17,22 @@
  #define mpxs_APR__Table_DELETE  apr_table_unset
  #define mpxs_APR__Table_CLEAR   apr_table_clear

+static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
+{
+    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
+    apr_table_t *t = apr_table_make(p, nelts);
+    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
+    sv_dump(SvRV(p_sv));
+    /* XXX: this seems to be ignored by perl 5.8.x+, since
+     * modperl_hash_tie already attached another _ext magic */
+#if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))
+    sv_magicext(SvRV(t_sv), p_sv, PERL_MAGIC_ext, NULL, Nullch, -1);
+#else
+    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
+#endif
+    return t_sv;
+}
+
  typedef struct {
      SV *cv;
      apr_hash_t *filter;



-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

>> +static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
>> +{
>> +    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>> +    apr_table_t *t = apr_table_make(p, nelts);
>> +    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
>> +    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>> +    return t_sv;
>> +}
>
> And that just happened to work, since it wasn't 5.8.x+
>
> sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>
> can't be used since it's already used by:
>
> MP_INLINE SV *modperl_hash_tie(pTHX_
> [...]
>
>      /* Prefetch magic requires perl 5.8 */
> #if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))
>
>      sv_magic(hv, NULL, PERL_MAGIC_ext, Nullch, -1);
>      SvMAGIC(hv)->mg_virtual = (MGVTBL *)&modperl_table_magic_prefetch;
>      SvMAGIC(hv)->mg_flags |= MGf_COPY;
>
> #endif /* End of prefetch magic */
>
>      sv_magic(hv, rsv, PERL_MAGIC_tied, Nullch, 0);
>
> so it happened to worked before I guess because I was testing with 5.6.x,
>
> with 5.8.x, if I dump the table object it has only one _ext magic.
>
> so we need to use some other magic to create this dependency.

You probably just need to use sv_magicext with 5.8.x, because
sv_magic doesn't seem to permit duplicates.  The only issue
then is ordering: you want the mpxs_APR__Table_make one further 
down the SvMAGIC chain than the modperl_hash_tie one.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
> +static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
> +{
> +    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
> +    apr_table_t *t = apr_table_make(p, nelts);
> +    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
> +    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
> +    return t_sv;
> +}

And that just happened to work, since it wasn't 5.8.x+

sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);

can't be used since it's already used by:

MP_INLINE SV *modperl_hash_tie(pTHX_
[...]

     /* Prefetch magic requires perl 5.8 */
#if ((PERL_REVISION == 5) && (PERL_VERSION >= 8))

     sv_magic(hv, NULL, PERL_MAGIC_ext, Nullch, -1);
     SvMAGIC(hv)->mg_virtual = (MGVTBL *)&modperl_table_magic_prefetch;
     SvMAGIC(hv)->mg_flags |= MGf_COPY;

#endif /* End of prefetch magic */

     sv_magic(hv, rsv, PERL_MAGIC_tied, Nullch, 0);

so it happened to worked before I guess because I was testing with 5.6.x,

with 5.8.x, if I dump the table object it has only one _ext magic.

so we need to use some other magic to create this dependency.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
>>Joe Schaefer wrote:
>>
>>>Perhaps this effort would be a good candidate for
>>>a short-lived "pool-scope-2.x-unstable" branch.
> 
> 
> [...]
> 
> 
>>You suggest the branch because we aren't sure that's the way we will
>>do it? Or is it because instead of sending patches one can freely
>>commit things and get others the changes right away?
> 
> 
> Yes-  I hate seeing good efforts stall out while waiting 
> for consensus to form.

Well if there is no consensus then why wasting time in first place.

I guess we have to wait until after the thankslazygivings to be over and 
hopefully will hear from the others.

>>Otherwise what's wrong with gradual changes, starting with APR::Table
>>and moving to other classes and refactoring on the way...
> 
> 
> Nothing, if there's ready consensus that your 
> APR::Table patch is the right approach. Of course
> you have my +1 to commit it to trunk and whittle away 
> at your heart's content.
 >
>>In any case I hope that others will comment on the problem in
>>hand and give their insights on the proposed solutions...
>  
> Right, but in the event that doesn't happen before 
> you've lost your zeal, I suggest making a branch
> before simply giving up on it.

I won't lose it since it's a showstopper for 2.0 release. And I want to 
see 2.0 RC1 out really soon now.

But let's do the branch at least in order to learn how branching works 
with svn.


-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> Joe Schaefer wrote:
>> Perhaps this effort would be a good candidate for
>> a short-lived "pool-scope-2.x-unstable" branch.

[...]

> You suggest the branch because we aren't sure that's the way we will
> do it? Or is it because instead of sending patches one can freely
> commit things and get others the changes right away?

Yes-  I hate seeing good efforts stall out while waiting 
for consensus to form.

> Otherwise what's wrong with gradual changes, starting with APR::Table
> and moving to other classes and refactoring on the way...

Nothing, if there's ready consensus that your 
APR::Table patch is the right approach. Of course
you have my +1 to commit it to trunk and whittle away 
at your heart's content.

> In any case I hope that others will comment on the problem in
> hand and give their insights on the proposed solutions...

Right, but in the event that doesn't happen before 
you've lost your zeal, I suggest making a branch
before simply giving up on it.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
>>We need to figure out how to automate this.
> 
> 
> Perhaps this effort would be a good candidate for
> a short-lived "pool-scope-2.x-unstable" branch.  
> Since we may need a widespread fix, a few of us 
> can work together on that branch and quickly merge 
> it back to trunk (once everything is sorted out).

You suggest the branch because we aren't sure that's the way we will do 
it? Or is it because instead of sending patches one can freely commit 
things and get others the changes right away?

Otherwise what's wrong with gradual changes, starting with APR::Table and 
moving to other classes and refactoring on the way...

In any case I hope that others will comment on the problem in hand and 
give their insights on the proposed solutions...

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> We need to figure out how to automate this.

Perhaps this effort would be a good candidate for
a short-lived "pool-scope-2.x-unstable" branch.  
Since we may need a widespread fix, a few of us 
can work together on that branch and quickly merge 
it back to trunk (once everything is sorted out).

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
[...]
> So trying to go the way Joe has chosen for apreq, I've replaced the 
> plain apr_table_make autogenerated wrapper with explicit:
> 
> static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
> {
>     apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
>     apr_table_t *t = apr_table_make(p, nelts);
>     SV *t_sv = mp_xs_APR__Table_2obj(t);
>     sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
>     return t_sv;
> }

That code wasn't correct, the good one is:

Index: xs/maps/apr_functions.map
===================================================================
--- xs/maps/apr_functions.map	(revision 106595)
+++ xs/maps/apr_functions.map	(working copy)
@@ -244,7 +244,8 @@
  MODULE=APR::Table
   apr_table_clear
   apr_table_copy    | | t, p
- apr_table_make
+~apr_table_make
+ mpxs_APR__Table_make
   apr_table_overlap
   apr_table_overlay | | base, overlay, p
   apr_table_compress
Index: xs/APR/Table/APR__Table.h
===================================================================
--- xs/APR/Table/APR__Table.h	(revision 106595)
+++ xs/APR/Table/APR__Table.h	(working copy)
@@ -17,6 +17,17 @@
  #define mpxs_APR__Table_DELETE  apr_table_unset
  #define mpxs_APR__Table_CLEAR   apr_table_clear

+
+static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
+{
+    apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
+    apr_table_t *t = apr_table_make(p, nelts);
+    SV *t_sv = modperl_hash_tie(aTHX_ "APR::Table", Nullsv, t);
+    sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
+    return t_sv;
+}
+
+
  typedef struct {
      SV *cv;
      apr_hash_t *filter;

We need to figure out how to automate this.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Stas Bekman wrote:
> I've rebuit with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", but 
> also apr1/httpd2.1
> 
>>>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>>>   $t= APR::Table::make(APR::Pool->new, 10);   $t->set($_=>$_), print 
>>> "Set $_" for 1..20'
>>>
>>>   Segmentation fault
> 
> 
> Now I get the segfault

So trying to go the way Joe has chosen for apreq, I've replaced the plain 
apr_table_make autogenerated wrapper with explicit:

static MP_INLINE SV *mpxs_APR__Table_make(pTHX_ SV *p_sv, int nelts)
{
     apr_pool_t *p = mp_xs_sv2_APR__Pool(p_sv);
     apr_table_t *t = apr_table_make(p, nelts);
     SV *t_sv = mp_xs_APR__Table_2obj(t);
     sv_magic(SvRV(t_sv), p_sv, PERL_MAGIC_ext, Nullch, -1);
     return t_sv;
}

and segfault is gone. here we create a dependency p_sv <- t_sv, so only 
when t_sv goes out of scope the pool object will be destroyed, and this 
code is now safe:

   APR::Table::make(APR::Pool->new, 10);

Unfortunately it doesn't seem like this dependency code can be easily 
integrated with the automatic type conversion, because the code accepts a 
single argument in xs/modperl_xs_sv_convert.h.

At the moment we will need to rewrite all methods that accept the pool 
object, like I did for APR::Table::make above. for example inside 
APR::Table there are also copy() and overlay() that need the same solution.

Any suggestions?

I'm thinking whether we could use the same solution for PV return values 
that use the pool object. For example with server_root_relative(), see the 
end of notes at:
http://perl.apache.org/docs/2.0/api/Apache/ServerUtil.html#C_server_root_relative_
which documents the danger of having the SvPV having a hole in body if the 
pool went out of scope. I guess we can attach the same magical dependency 
there too.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
I've rebuit with --enable-pool-debug CPPFLAGS="-DAPR_BUCKET_DEBUG", but 
also apr1/httpd2.1

>>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>>   $t= APR::Table::make(APR::Pool->new, 10);   $t->set($_=>$_), print 
>> "Set $_" for 1..20'
>>
>>   Segmentation fault

Now I get the segfault

#0  0x40392d66 in apr_table_set (t=0x8067fc0, key=0x805f040 "2",
     val=0x805f040 "2") at tables/apr_tables.c:531
531         next_elt->key = apr_pstrdup(t->a.pool, key);
(gdb) bt
#0  0x40392d66 in apr_table_set (t=0x8067fc0, key=0x805f040 "2",
     val=0x805f040 "2") at tables/apr_tables.c:531
#1  0x404e7bb4 in XS_APR__Table_set (my_perl=0x804c008, cv=0x813b648)
     at Table.c:243
#2  0x400bb649 in Perl_pp_entersub (my_perl=0x804c008) at pp_hot.c:2890
#3  0x40097a4d in Perl_runops_debug (my_perl=0x804c008) at dump.c:1449
#4  0x4003bead in S_run_body (my_perl=0x804c008, oldscope=1) at perl.c:1934
#5  0x4003b8ed in perl_run (my_perl=0x804c008) at perl.c:1853
#6  0x080494c6 in main (argc=2, argv=0xbffff374, env=0xbffff380)
     at perlmain.c:98

Great!

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
>>In any case I'm pretty sure that you've already thought about a
>>great solution and you have an almost working patch. Don't say that
>>you don't have one :) 
> 
> 
> The solution IMO, is to do what Apache::Request::Table::new does:
> keep a reference to the APR::Pool argument in the new APR::Table
> object's magic list.  That ensures the pool will last as long as 
> the table SV does.
> 
> I'm pretty sure I proposed this idea before, but it met with 
> some resistance.  IIRC folks here considered this a bug in
> APR::Pool, but I say the problem here is in APR::Table.  I'd
> like to see us settle on a course of action first.

Yeah, I saw your changes to Apache::Request::Table and was planning to 
look at that. The problem with your solution is that it'll require getting 
this code into any function that gets the pool object. And there are quite 
a few of those. That's why we need to look first at the possibility to 
have the pool know when it's safe to go away.

I haven't yet studied in detail the latest reincarnation of APR::Pool, but 
the previous implementation was refcounting every time a pool object was 
asked for. And it won't destroy till all the users cease to exist. 
Granted, this worked (sort of) for pool objects, but it didn't solve the 
problem we are discussing now. So I wonder if it's possible to somehow 
store the users of the pool (svs) (and refcount them) in the pool and not 
the other way around. It's probably not quite possible, w/o invoking some 
code every time a pool is a passed to method/func, which will essentially 
do the same as your code in Apache::Request::Table::new.

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> In any case I'm pretty sure that you've already thought about a
> great solution and you have an almost working patch. Don't say that
> you don't have one :) 

The solution IMO, is to do what Apache::Request::Table::new does:
keep a reference to the APR::Pool argument in the new APR::Table
object's magic list.  That ensures the pool will last as long as 
the table SV does.

I'm pretty sure I proposed this idea before, but it met with 
some resistance.  IIRC folks here considered this a bug in
APR::Pool, but I say the problem here is in APR::Table.  I'd
like to see us settle on a course of action first.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:

>>They don't quite segfault for me. How is your system different than mine?
> 
> 
> The major differences:
> 
> I'm testing 64bit debian on the trunk of all projects (httpd, apr, mp2)
> against a fresh perl-5.8.x snapshot, and I enabled every debugging option
> I could think of (--enable-pool-debug, CFLAGS="-g3 -gdwarf-2 -O2",
> CPPFLAGS="-DAPR_BUCKET_DEBUG"; MP_DEBUG=1; MP_MAINTAINER=1) when 
> compiling them.

>>   MP_USE_GTOP     => 1

> I don't have gtop enabled.

most likely irrelevant. it's only good for memory usage stats and tracing 
memory leaks.

>>*** (apr|apu)-config linking info
> 
> 
> Pretty much the same as mine, but with -lapr-1 and -laprutil-1.  This
> is a factor, because the segfaults are coming from libapr:
> 
> % bt
> #0  apr_table_set (t=0x5236b0, key=0x517960 "1", val=0x517960 "1")
>     at tables/apr_tables.c:531
> #1  0x0000002a96b6eb02 in XS_APR__Table_set (my_perl=0x504010, cv=0x517960)
>     at Table.c:243
> #2  0x0000002a95704a43 in Perl_pp_entersub (my_perl=0x504010) at pp_hot.c:2890
> #3  0x0000002a956ea40e in Perl_runops_debug (my_perl=0x504010) at dump.c:1449
> #4  0x0000002a956a40e0 in S_run_body (my_perl=0x504010, oldscope=1)
>     at perl.c:1934
> #5  0x0000002a956a3e45 in perl_run (my_perl=0x504010) at perl.c:1853
> #6  0x0000000000401aff in main (argc=6, argv=0x7fbffff8f8, env=0x2)
>     at perlmain.c:95
> 
> % p *t->a.pool
> $2 = {parent = 0x0, child = 0x0, sibling = 0x5236a0, ref = 0x5236a0,
>   cleanups = 0x2a961ed288, free_cleanups = 0x2a961ed288,
>   allocator = 0x2a961ed298, subprocesses = 0x2a961ed298, abort_fn = 0x5b10b0,
>   user_data = 0x5094c0, tag = 0x2a961ed2b8 "°\020[", nodes = 0x2a961ed2b8,
>   file_line = 0x599ad0 "\220FR", creation_flags = 5338784, stat_alloc = 0,
>   stat_total_alloc = 2518602456, stat_clear = 42, owner = 182907228888,
>   mutex = 0x568b40}

May be if I try to build with --enable-pool-debug 
CPPFLAGS="-DAPR_BUCKET_DEBUG" I'll trigger those with apr-0 as well.

I doubt that CFLAGS="-g3 -gdwarf-2 -O2" has anything do with this. It 
creates a way too big libs, which makes things slow, even for debug. You 
want those flags only if you want to be able to expand macros in gdb. I'm 
not sure what else are those good for.

thanks for the info, Joe.

In any case I'm pretty sure that you've already thought about a great 
solution and you have an almost working patch. Don't say that you don't 
have one :)

-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> Joe Schaefer wrote:
>
>>>Do you have any tests that clearly demonstrate the problem?
>> Guaranteeing a segfault may be tricky,
>
> They don't quite segfault for me. How is your system different than mine?

The major differences:

I'm testing 64bit debian on the trunk of all projects (httpd, apr, mp2)
against a fresh perl-5.8.x snapshot, and I enabled every debugging option
I could think of (--enable-pool-debug, CFLAGS="-g3 -gdwarf-2 -O2",
CPPFLAGS="-DAPR_BUCKET_DEBUG"; MP_DEBUG=1; MP_MAINTAINER=1) when 
compiling them.

> *** Makefile.PL options:

[...]

>    MP_USE_GTOP     => 1
     ^^^^^^^^^^^^^^^^^^^^

I don't have gtop enabled.


[...]

> *** (apr|apu)-config linking info

Pretty much the same as mine, but with -lapr-1 and -laprutil-1.  This
is a factor, because the segfaults are coming from libapr:

% bt
#0  apr_table_set (t=0x5236b0, key=0x517960 "1", val=0x517960 "1")
    at tables/apr_tables.c:531
#1  0x0000002a96b6eb02 in XS_APR__Table_set (my_perl=0x504010, cv=0x517960)
    at Table.c:243
#2  0x0000002a95704a43 in Perl_pp_entersub (my_perl=0x504010) at pp_hot.c:2890
#3  0x0000002a956ea40e in Perl_runops_debug (my_perl=0x504010) at dump.c:1449
#4  0x0000002a956a40e0 in S_run_body (my_perl=0x504010, oldscope=1)
    at perl.c:1934
#5  0x0000002a956a3e45 in perl_run (my_perl=0x504010) at perl.c:1853
#6  0x0000000000401aff in main (argc=6, argv=0x7fbffff8f8, env=0x2)
    at perlmain.c:95

% p *t->a.pool
$2 = {parent = 0x0, child = 0x0, sibling = 0x5236a0, ref = 0x5236a0,
  cleanups = 0x2a961ed288, free_cleanups = 0x2a961ed288,
  allocator = 0x2a961ed298, subprocesses = 0x2a961ed298, abort_fn = 0x5b10b0,
  user_data = 0x5094c0, tag = 0x2a961ed2b8 "°\020[", nodes = 0x2a961ed2b8,
  file_line = 0x599ad0 "\220FR", creation_flags = 5338784, stat_alloc = 0,
  stat_total_alloc = 2518602456, stat_clear = 42, owner = 182907228888,
  mutex = 0x568b40}


> *** /home/stas/perl/5.8.6-ithread/bin/perl5.8.6 -V
> Summary of my perl5 (revision 5 version 8 subversion 6) configuration:
>    Platform:
>      osname=linux, osvers=2.6.8.1-12mdk, archname=i686-linux-thread-multi
>      uname='linux rabbit.stason.org 2.6.8.1-12mdk #1 fri oct 1 12:53:41 cest
> 2004 i686 mobile intel(r) pentium(r) 4 - m cpu 2.00ghz unknown gnulinux '
>      config_args='-des -Dprefix=/home/stas/perl/5.8.6-ithread -Dusethreads
> -Doptimize=-g -Duseshrplib -Dusedevel -Accflags=-DDEBUG_LEAKING_SCALARS'
>      hint=recommended, useposix=true, d_sigaction=define
>      usethreads=define use5005threads=undef useithreads=define
> usemultiplicity=define
>      useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
>      use64bitint=undef use64bitall=undef uselongdouble=undef
>      usemymalloc=n, bincompat5005=undef
>    Compiler:
>      cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS
> -DDEBUG_LEAKING_SCALARS -DDEBUGGING -fno-strict-aliasing -pipe
> -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64
> -I/usr/include/gdbm',
>      optimize='-g',
>      cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS
> -DDEBUG_LEAKING_SCALARS -DDEBUGGING -fno-strict-aliasing -pipe
> -I/usr/local/include -I/usr/include/gdbm'
>      ccversion='', gccversion='3.4.1 (Mandrakelinux 10.1 3.4.1-4mdk)',
> gccosandvers=''
>      intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
>      d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
>      ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t',
> lseeksize=8
>      alignbytes=4, prototype=define
>    Linker and Libraries:
>      ld='cc', ldflags =' -L/usr/local/lib'
>      libpth=/usr/local/lib /lib /usr/lib
>      libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
>      perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
>      libc=/lib/libc-2.3.3.so, so=so, useshrplib=true, libperl=libperl.so
>      gnulibc_version='2.3.3'
>    Dynamic Linking:
>      dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
> -Wl,-rpath,/home/stas/perl/5.8.6-ithread/lib/5.8.6/i686-linux-thread-multi/CORE'
>      cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'

*** /home/joe/perl/5.8.x/bin/perl -V
Summary of my perl5 (revision 5 version 8 subversion 6) configuration:
  Platform:
    osname=linux, osvers=2.6.9, archname=x86_64-linux-thread-multi
    uname='linux gemini 2.6.9 #2 smp thu nov 4 21:00:37 est 2004 x86_64 gnulinux '
    config_args='-Dprefix=/home/joe/perl/5.8.x -Dusethreads -Duseshrplib
    -Doptimize=-g3 -gdwarf-2 -O2 -des' 
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=define use64bitall=define uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS
    -DDEBUGGING -fno-strict-aliasing -pipe -I/usr/local/include
    -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64', 
    optimize='-g3 -gdwarf-2 -O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBUGGING
    -fno-strict-aliasing -pipe -I/usr/local/include' 
    ccversion='', gccversion='3.3.5 (Debian 1:3.3.5-2)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
    libc=/lib/libc-2.3.2.so, so=so, useshrplib=true, libperl=libperl.so
    gnulibc_version='2.3.2'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E
    -Wl,-rpath,/home/joe/perl/5.8.x/lib/5.8.6/x86_64-linux-thread-multi/CORE' 
    cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'


> Characteristics of this binary (from libperl):
>    Compile-time options: DEBUGGING MULTIPLICITY USE_ITHREADS USE_LARGE_FILES
> PERL_IMPLICIT_CONTEXT
>    Locally applied patches:
>          MAINT23477
>    Built under linux
>    Compiled at Nov 23 2004 11:38:42
>    %ENV:
>      PERLDOC_PAGER="less -R"
>      PERL_LWP_USE_HTTP_10="1"
>    @INC:
>      /home/stas/perl/5.8.6-ithread/lib/5.8.6/i686-linux-thread-multi
>      /home/stas/perl/5.8.6-ithread/lib/5.8.6
>      /home/stas/perl/5.8.6-ithread/lib/site_perl/5.8.6/i686-linux-thread-multi
>      /home/stas/perl/5.8.6-ithread/lib/site_perl/5.8.6
>      /home/stas/perl/5.8.6-ithread/lib/site_perl


Characteristics of this binary (from libperl):
  Compile-time options: DEBUGGING MULTIPLICITY USE_ITHREADS
  USE_64_BIT_INT USE_64_BIT_ALL USE_LARGE_FILES PERL_IMPLICIT_CONTEXT 
  Locally applied patches:
        MAINT23477
  Built under linux
  Compiled at Nov 24 2004 01:32:52
  %ENV:
    PERL_LWP_USE_HTTP_10="1"
  @INC:
    /home/joe/perl/5.8.x/lib/5.8.6/x86_64-linux-thread-multi
    /home/joe/perl/5.8.x/lib/5.8.6
    /home/joe/perl/5.8.x/lib/site_perl/5.8.6/x86_64-linux-thread-multi
    /home/joe/perl/5.8.x/lib/site_perl/5.8.6
    /home/joe/perl/5.8.x/lib/site_perl
    .
-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:

>>Do you have any tests that clearly demonstrate the problem?
> 
> 
> Guaranteeing a segfault may be tricky, 

They don't quite segfault for me. How is your system different than mine? 
See my t/REPORT at the end of this email

> but here are few command-line 
> examples that work consistently for me:
> 
>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle ' 
>   {
>     my $p = APR::Pool->new; $p->cleanup_register(sub {print "Gone"}); 
>     $t = APR::Table::make($p, 10) 
>   }; print "Left block"; $t->set(a=>5); print $t->get("a")'
> 
>   Gone
>   Left block
>   Segmentation fault

perl-5.8.6-ithread -Mblib -MApache2 -MAPR::Pool -MAPR::Table -wl
  {
     my $p = APR::Pool->new; $p->cleanup_register(sub {print "Gone"});
     $t = APR::Table::make($p, 10)
   }; print "Left block"; $t->set(a=>5); print $t->get("a")
Gone
Left block
5

>   % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>   $t= APR::Table::make(APR::Pool->new, 10); 
>   $t->set($_=>$_), print "Set $_" for 1..20'
> 
>   Segmentation fault

Set 1
[...]
Set 20



> And as dumb luck would have it, here's what your specific
> example did for me:
> 
>     % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
>     $t = APR::Table::make(APR::Pool->new, 10); 
>     $t->set(a=>5); print "Set ok"; print $t->get("a")'
> 
>     Set ok
>     5

2. Used Components and their Configuration:

*** mod_perl version 1.9918

*** using /home/stas/apache.org/mp2-svn/lib/Apache/BuildConfig.pm

*** Makefile.PL options:
   MP_APR_LIB      => aprext
   MP_APXS         => /home/stas/httpd/prefork/bin/apxs
   MP_CCOPTS       => -Wall -Werror
   MP_COMPAT_1X    => 1
   MP_DEBUG        => 1
   MP_GENERATE_XS  => 1
   MP_INST_APACHE2 => 1
   MP_LIBNAME      => mod_perl
   MP_MAINTAINER   => 1
   MP_TRACE        => 1
   MP_USE_DSO      => 1
   MP_USE_GTOP     => 1


*** /home/stas/httpd/prefork/bin/httpd -V
Server version: Apache/2.0.53-dev
Server built:   Sep 23 2004 23:20:50
Server's Module Magic Number: 20020903:9
Architecture:   32-bit
Server compiled with....
  -D APACHE_MPM_DIR="server/mpm/prefork"
  -D APR_HAS_SENDFILE
  -D APR_HAS_MMAP
  -D APR_HAVE_IPV6 (IPv4-mapped addresses enabled)
  -D APR_USE_SYSVSEM_SERIALIZE
  -D APR_USE_PTHREAD_SERIALIZE
  -D SINGLE_LISTEN_UNSERIALIZED_ACCEPT
  -D APR_HAS_OTHER_CHILD
  -D AP_HAVE_RELIABLE_PIPED_LOGS
  -D HTTPD_ROOT="/home/stas/httpd/prefork"
  -D SUEXEC_BIN="/home/stas/httpd/prefork/bin/suexec"
  -D DEFAULT_PIDLOG="logs/httpd.pid"
  -D DEFAULT_SCOREBOARD="logs/apache_runtime_status"
  -D DEFAULT_LOCKFILE="logs/accept.lock"
  -D DEFAULT_ERRORLOG="logs/error_log"
  -D AP_TYPES_CONFIG_FILE="conf/mime.types"
  -D SERVER_CONFIG_FILE="conf/httpd.conf"


*** (apr|apu)-config linking info

  -L/home/stas/httpd/prefork/lib -lapr-0 -lrt -lm -lcrypt -lnsl  -lpthread 
-ldl
  -L/home/stas/httpd/prefork/lib -laprutil-0 -lgdbm -ldb-4.0 -lexpat



*** /home/stas/perl/5.8.6-ithread/bin/perl5.8.6 -V
Summary of my perl5 (revision 5 version 8 subversion 6) configuration:
   Platform:
     osname=linux, osvers=2.6.8.1-12mdk, archname=i686-linux-thread-multi
     uname='linux rabbit.stason.org 2.6.8.1-12mdk #1 fri oct 1 12:53:41 
cest 2004 i686 mobile intel(r) pentium(r) 4 - m cpu 2.00ghz unknown gnulinux '
     config_args='-des -Dprefix=/home/stas/perl/5.8.6-ithread -Dusethreads 
-Doptimize=-g -Duseshrplib -Dusedevel -Accflags=-DDEBUG_LEAKING_SCALARS'
     hint=recommended, useposix=true, d_sigaction=define
     usethreads=define use5005threads=undef useithreads=define 
usemultiplicity=define
     useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
     use64bitint=undef use64bitall=undef uselongdouble=undef
     usemymalloc=n, bincompat5005=undef
   Compiler:
     cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS 
-DDEBUG_LEAKING_SCALARS -DDEBUGGING -fno-strict-aliasing -pipe 
-I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 
-I/usr/include/gdbm',
     optimize='-g',
     cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS 
-DDEBUG_LEAKING_SCALARS -DDEBUGGING -fno-strict-aliasing -pipe 
-I/usr/local/include -I/usr/include/gdbm'
     ccversion='', gccversion='3.4.1 (Mandrakelinux 10.1 3.4.1-4mdk)', 
gccosandvers=''
     intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
     d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=12
     ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', 
lseeksize=8
     alignbytes=4, prototype=define
   Linker and Libraries:
     ld='cc', ldflags =' -L/usr/local/lib'
     libpth=/usr/local/lib /lib /usr/lib
     libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lpthread -lc
     perllibs=-lnsl -ldl -lm -lcrypt -lutil -lpthread -lc
     libc=/lib/libc-2.3.3.so, so=so, useshrplib=true, libperl=libperl.so
     gnulibc_version='2.3.3'
   Dynamic Linking:
     dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E 
-Wl,-rpath,/home/stas/perl/5.8.6-ithread/lib/5.8.6/i686-linux-thread-multi/CORE'
     cccdlflags='-fpic', lddlflags='-shared -L/usr/local/lib'


Characteristics of this binary (from libperl):
   Compile-time options: DEBUGGING MULTIPLICITY USE_ITHREADS 
USE_LARGE_FILES PERL_IMPLICIT_CONTEXT
   Locally applied patches:
         MAINT23477
   Built under linux
   Compiled at Nov 23 2004 11:38:42
   %ENV:
     PERLDOC_PAGER="less -R"
     PERL_LWP_USE_HTTP_10="1"
   @INC:
     /home/stas/perl/5.8.6-ithread/lib/5.8.6/i686-linux-thread-multi
     /home/stas/perl/5.8.6-ithread/lib/5.8.6
     /home/stas/perl/5.8.6-ithread/lib/site_perl/5.8.6/i686-linux-thread-multi
     /home/stas/perl/5.8.6-ithread/lib/site_perl/5.8.6
     /home/stas/perl/5.8.6-ithread/lib/site_perl
     .

*** Packages of interest status:

Apache::Request: -
CGI            : 3.05
LWP            : 5.800
mod_perl       : 1.2901, 1.9918



-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> which patch are you talking about Joe, the one that was committed

Yes.

[...]

> Do you have any tests that clearly demonstrate the problem?

Guaranteeing a segfault may be tricky, but here are few command-line 
examples that work consistently for me:

  % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle ' 
  {
    my $p = APR::Pool->new; $p->cleanup_register(sub {print "Gone"}); 
    $t = APR::Table::make($p, 10) 
  }; print "Left block"; $t->set(a=>5); print $t->get("a")'

  Gone
  Left block
  Segmentation fault



  % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
  $t= APR::Table::make(APR::Pool->new, 10); 
  $t->set($_=>$_), print "Set $_" for 1..20'

  Segmentation fault



And as dumb luck would have it, here's what your specific
example did for me:

    % ~/perl/5.8.x/bin/perl -MApache2 -MAPR::Pool -MAPR::Table -wle '
    $t = APR::Table::make(APR::Pool->new, 10); 
    $t->set(a=>5); print "Set ok"; print $t->get("a")'

    Set ok
    5

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Stas Bekman <st...@stason.org>.
Joe Schaefer wrote:
> Stas Bekman <st...@stason.org> writes:
> 
> 
>>So does that meant that the latest reincarnation of APR::Pool
>>implementation has this problem fixed?
> 
> 
> If you're talking about my APR::Pool patch, then no,
> it didn't resolve this.  The underlying apr_pool_t 
> will be destroyed by APR::Pool::DESTROY in your examples,
> so using any pool-derived objects after that event is totally 
> unsafe.  If it doesn't segfault, either you're quite lucky
> or there's a bug in my patch.

which patch are you talking about Joe, the one that was committed or else?

Do you have any tests that clearly demonstrate the problem? I was 
surprised that my previous tests all worked just fine. I want to start 
with a failing test before trying to fix it.



-- 
__________________________________________________________________
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: [mp2] pools that go out of scope aren't a problem anymore?

Posted by Joe Schaefer <jo...@sunstarsys.com>.
Stas Bekman <st...@stason.org> writes:

> So does that meant that the latest reincarnation of APR::Pool
> implementation has this problem fixed?

If you're talking about my APR::Pool patch, then no,
it didn't resolve this.  The underlying apr_pool_t 
will be destroyed by APR::Pool::DESTROY in your examples,
so using any pool-derived objects after that event is totally 
unsafe.  If it doesn't segfault, either you're quite lucky
or there's a bug in my patch.

-- 
Joe Schaefer


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org