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

Re: [BUG] Mutex is destroyed when clearing top level pool, WAS: RE: svn commit: rev 2017 - trunk/subversion/libsvn_subr

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2002-05-30 01:40:39 CEST

"Sander Striker" <striker@apache.org> writes:

> > From: Philip Martin
> > Sent: 29 May 2002 23:59
>
> > "Sander Striker" <striker@apache.org> writes:
> >> There is a difference between debug and production. Your patch
> >> only addressed the debug version.
> >
> > I don't understand. There is no bug in the production version, the
> > mutex in the pool is not even created.
>
> Look at svn_pool_create in libsvn_subr. We create a mutex in the
> top level subversion pool, for its own allocator. Clear that pool
> and reuse it... BOOM.

I am not interested in Subversion at the moment, I am trying to fix
APR. The debug APR has a bug. The production APR doesn't have the
same bug because it doesn't use the same mutex (it may have a totally
separate bug, I haven't looked). Thus my patch only addresses the
debug APR build. That's what I want to fix.

[snip]

> There is a difference between accessing the 'pool tree' and 'using
> a pool'. If you are doing clear/destroy in one thread and p[c]alloc
> in another on the same pool, this is very likely to blow up.
>
> The mutex is there so that multiple threads can create/destroy child
> pools on a shared pool.

(And alloc from, and clear, the child pools. Not much point creating
child pools if one can't use them. But only one thread *allocates*
from a pool unless there is additional synchronisation.)

Your patch doesn't allow this. Destroying a child pool calls clear on
the pool. Calling clear destroys the mutex without checking whether
any other thread has it locked or is waiting to lock it. It's not
thread-safe.

>
> In case it isn't obvious by now: pools aren't thread safe. You should
> never share a pool between threads if the threads aren't synchronized.

I don't expect to be able to allocate from a single pool in multiple
threads. I expect to share the pool parent-child hierarchy between
threads, as described above. To do that the mutex must never be
destroyed while a pool is within its parent's list of children. The
mutex must either exist for the lifetime of the pool, as per my
original patch, or the pool must be disconnected from the hierarchy
before destroying and recreating the mutex, as in the algorithm I
outlined.

Here's a scenario, time passes down the page,

   thread_a start given pool_1
   alloc from pool_1
   ...
   create pool_2 from pool_1
   start thread_b passing pool_2
   ... thread_b start given pool_2
   ... create pool_3 from pool_2
   ... alloc from pool_3
   alloc from pool_1 ...
   ... clear pool_3
   ... alloc from pool_3
   create pool_4 ...
   alloc from pool_4 ...
   destroy pool_4 ...
   ... destroy pool_3
   wait for thread_b thread_b end
   ...
   destroy pool_2
   thread_a end

Every time thread_a calls a function that does apr_pool_walk_tree (say
apr_palloc calling apr_pool_log_event calling apr_pool_num_bytes
calling apr_pool_walk_tree) it will attempt to lock the pool_3 mutex.
When thread_b calls clear or destroy on pool_3 the mutex will get
destroy. No account is taken of the possibility that thread_a is
using the mutex.

-- 
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Jun 1 14:22:40 2002

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.