Re: CVS update: subversion/subversion/libsvn_delta svndiff.c

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/