Joe Schaefer wrote:
Stas Bekman <[EMAIL PROTECTED]> writes:

[...]


There are going to be various tricky bugs to figure out with the new
things. I just had one when I tried:

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

it took me time to figure out that the pool has gone out of scope and
was destroyed by the time read was attempted.


Yup- it took me a few reads to even understand the problem
you had :-).

:)

Scope of pools is a tricky thing. So all the APIs relying on a user providing the pool explicitly are potentially problematic at the moment.

This fixed the problem:

perl -MApache2 -MAPR::Pool -MAPR::PerlIO -le '; my $p =
APR::Pool->new; open my $fh, "<:APR", "/tmp/xxx", $p; print <$fh>'

tricky. We may need to have much more defending code if we don't want
to get a rain of bug reports. And writing it is not trivial at all :(

so in the case of open() we probably need to increment the refcount
of the pool object and decrement it on close and hope that we don't
get a leak anywhere.


The problem is the same for any pool-derived object,
e.g.


  #! perl
  use Apache2;
  use APR::Pool;
  use Apache::Request;
  my $req = Apache::Request->new(APR::Pool->new);
  print $req->param; #KaBoom!

In apreq we don't currently tag the pool SV that constructed us,
so a DESTROY method can't decrement it. We could change that in apreq2,but doing that throughout mp2 might be messy.

It'd be nice to have a common way to do that. Making this manually for each object is going to be a big mess and error-prone.


Is it possible to do something within APR::Pool to address this? Maybe calling APR::Pool->new()
can dish out pool objects from an internal list:


  package APR::Pool;
  sub new {
        my $parent = shift;
        if (!ref $parent) {
            my $pool = <new APR::Pool object>;
            push @INTERNAL_POOL_LIST, $pool;
            return $pool;
        }
  ...

  sub DESTROY {
        my $pool = shift;
        if (!ref $pool) {
            pop @INTERNAL_POOL_LIST;
        }
  }


Here somebody would need to call APR::Pool->DESTROY
to actually relinquish the last pool.

Only the last one? I'm not quite sure how are you going to clean up the pool of pools. How do you know that you pop the right pool?


What really needs to happen is to make the $pool object (or the temp) be tied to the object it belongs to ($apreq, $r, etc) and automatically get its refcount to 0 when the corresponding parent object goes out of scope. Which I suppose can be done with a simple DESTROY method. So when we accept the pool object we bump up its ref count and when DESTROY is called it'll decrement this count (this is because there could be more than one object using the same pool).

I saw your commit -- if I understand it correctly, you try to make it dependent on the parent, so it won't get its refcount to 0 before the parent go away, right?

Conway's OOP
book discusses the "flyweight pattern", which probably has much better ideas than this.

It has been ages since I've read it, so I don't remember much of the neat tricks Damian gave there.



-- __________________________________________________________________ Stas Bekman JAm_pH ------> Just Another mod_perl Hacker http://stason.org/ mod_perl Guide ---> http://perl.apache.org mailto:[EMAIL PROTECTED] http://use.perl.org http://apacheweek.com http://modperlbook.org http://apache.org http://ticketmaster.com

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



Reply via email to