[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Allocating from a pool in cleanup, WAS: RE: svn commit: rev 3709 - in trunk/subversion: libsvn_wc libsvn_client

From: Sander Striker <striker_at_apache.org>
Date: 2002-11-10 23:25:59 CET

> From: Philip Martin [mailto:pm@localhost.striker.xs4all.nl]On Behalf Of
> Philip Martin
> Sent: 10 November 2002 17:10

> Philip Martin <philip@codematters.co.uk> writes:
>
>> The thing that worries me, something I have just recently thought of,
>> is that the parent pool could be the global pool. If that were the
>> case all the thread-safety problems would reappear.
>
> It doesn't have to be the global pool for this to be a threading
> problem. A thread cannot, in general, assume that it can safely
> access a parent pool, that only works if the thread has exclusive
> access to the parent pool.

Ouch! You're right.
 
> I'm not sure what to do here. We have gradually been moving away from
> functions that allocate their own pools, to functions that use the
> pools passed in to them. So svn_wc_adm_open creates an access baton
> in the pool it is given. When the cleanup handler for the access
> baton runs, it can choose to use either the pool it was given, or that
> pool's parent pool, and at present neither appears to be correct.
>
> Here's the best solution I can come up with: first we need an APR that
> allows us to allocate from the pool during pool cleanup.

We do have that, see below.

> Second we revert to using the pool, rather than the parent pool, in cleanup
> handlers.

Agreed, but currently with the limitation that no new cleanups should be
registered during cleanup.

> Finally, we change the Subversion access baton code, so
> that it registers its cleanup handler later, in particular it must
> register it *after* doing the first UTF8 conversion. This will ensure
> that even if the access baton is responsible for creating the
> apr_xlate_t object in the pool, the access baton cleanup handler will
> run *before* the APR handler cleans-up the apr_xlate_t object.
>
> I don't really understand the prohibition on allocating from a pool
> during a cleanup handler, Subversion has been doing it for some time
> and it doesn't appear to be a problem. I've had a quick look at the
> APR pool code and I see no obvious reason for it not to work (but I
> may well have missed something).

I can confirm that it is safe to do allocations from the pool being cleaned
up. The problem is registering cleanups in a pool in the process of being
cleaned up. Opening files, like a lot of other APR operations, registers
a cleanup with the pool being passed in. It probably wouldn't even be that
hard to allow registering cleanups during cleanup; I'll look into that.

> So working on the assumption that it is indeed OK, I made the other changes.
> FWIW, everything seems to work, the regression tests pass.

Great!

Sander

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sun Nov 10 23:12:49 2002

This is an archived mail posted to the Subversion Dev mailing list.