Greg, thanks for catching these and then changing APR to catch them
more often. If ever there was a Right Thing, that is it.
-K
Greg Stein <gstein@lyra.org> writes:
> I found this one with the help of the new pool debugging stuff. I saw a case
> in the debug output of a potential sequencing error. Specifically, most pool
> patterns will look like:
>
> create A
> create B
> free B
> free A
>
> I saw:
>
> create A
> create B
> free A
> free B
>
> While the above is quite possibly valid, it is certainly suspicious. Adding
> some info to the debug pool output, I verified the ordering problem.
>
> To assist with future ordering problems (automatically), I just checked in a
> change to apr_pools.c which trashes a pool structure at destroy() time (even
> in the non-debug case). That change caught the problem below.
>
> However, I think that I'm still seeing a problem on the server side of
> things, which I'm continuing to track down.
>
> Cheers,
> -g
>
> On Tue, Apr 17, 2001 at 10:51:48AM -0000, gstein@tigris.org wrote:
> > User: gstein
> > Date: 01/04/17 03:51:48
> >
> > Modified: subversion/libsvn_delta svndiff.c
> > Log:
> > Shut down the stream -> window mechanism in the correct order. Closing the
> > stream first could inadvertently throw out the parent of our subpool.
> >
> > (window_handler): swap order of pool destruction and stream closing.
> >
> > Revision Changes Path
> > 1.22 +16 -3 subversion/subversion/libsvn_delta/svndiff.c
> >
> > Index: svndiff.c
> > ===================================================================
> > RCS file: /cvs/subversion/subversion/libsvn_delta/svndiff.c,v
> > retrieving revision 1.21
> > retrieving revision 1.22
> > diff -u -r1.21 -r1.22
> > --- svndiff.c 2001/04/12 08:02:36 1.21
> > +++ svndiff.c 2001/04/17 10:51:48 1.22
> > @@ -117,10 +117,23 @@
> >
> > if (window == NULL)
> > {
> > - /* We're done; clean up. */
> > - err = svn_stream_close (eb->output);
> > + svn_stream_t *output = eb->output;
> > +
> > + /* We're done; clean up.
> > +
> > + We clean our pool first. Given that the output stream was passed
> > + TO us, we'll assume it has a longer lifetime, and that it will not
> > + be affected by our pool destruction.
> > +
> > + The contrary point of view (close the stream first): that could
> > + tell our user that everything related to the output stream is done,
> > + and a cleanup of the user pool should occur. However, that user
> > + pool could include the subpool we created for our work (eb->pool),
> > + which would then make our call to svn_pool_destory() puke.
> > + */
> > svn_pool_destroy (eb->pool);
> > - return err;
> > +
> > + return svn_stream_close (output);
> > }
> >
> > /* Encode the instructions. */
> >
> >
> >
>
> --
> Greg Stein, http://www.lyra.org/
Received on Sat Oct 21 14:36:28 2006