"Sander Striker" <striker@apache.org> writes:
> You should never use a pool after calling apr_pool_destroy or apr_pool_clear.
> Cleanups should not use the pool they are cleaning up in for new allocations
> or other operations.
Subversion is doing so in several places. What does "other
operations" mean? Can I call apr_pool_parent_get on a pointer to the
pool being destroyed?
> > Cleanups can't assume that their pools are still valid and have anything in
> > them
>
> correct.
Yikes! "can't assume [they] have anything in them" You mean memory
allocated from the pool may already be invalid? That makes the system
very hard to use :-(
> > (there is purposely no order to how cleanups in a pool are invoked -
>
> Last in, first out.
One sane thing.
> > this is to minimize what people do in a cleanup). So, the iconv_close()
> > has already been called by its pool cleanup function (registered by
> > apr_xlate). (Sander would know even more about the pool logic than I and
> > could confirm my hunch.)
> >
> > The reason reverting the patch works is that the iconv buffer isn't coming
> > from that pool, but instead from the global pool (which is never cleaned
> > up). I bet that on Solaris and Darwin, the iconv() buffers are stateless
> > (i.e. a close does nothing), or the pool_cleanup is called before the
> > iconv_close cleanup.
> >
> > You can't have any dependencies between pool cleanups. Either that pool
> > passed to svn_wc__adm_is_cleanup_required must be that pool's parent
> > (perhaps easiest), or pool_cleanup must go away.
>
> That are the options that came to my mind aswell.
Well, passing the parent doesn't help if the memory is invalid (see
above) as we won't be able to look at the access baton's stored path.
The path would need to be allocated from the parents pool. You don't
really mean that to cleanup an allocated structure I have to allocate
it from the parent pool?
I accept that attempting to add new cleanup handlers should fail, but
not being able to allocate new memory is unfortunate, and not being
able to read already allocated memory is disasterous!
>
> > Regardless, r3701 isn't at fault. It just indicated the faulty logic here.
> > =) -- justin
>
> I agree.
Well, in mitigation, I would point to the lack of documentation
describing these restrictions. This is not the first callback/cleanup
system I've used, but it appears to be the least powerful.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 9 03:16:38 2002