You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Sander Striker <st...@apache.org> on 2002/05/27 22:10:43 UTC

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

> From: Sander Striker [mailto:striker@apache.org]
> Sent: 27 May 2002 03:51

>> From: Philip Martin [mailto:pm@localhost]On Behalf Of Philip Martin
>> Sent: 27 May 2002 03:13
>> striker@tigris.org writes:
>> 
>>> Author: striker
>>> Date: 2002-05-26 09:27 GMT
>>> New Revision: 2017

[...]
>> Since this change the Subversion regression tests are failing when
>> compiled with APR_POOL_DEBUG=1.  They hang in svn_pool_clear() when it
>> recreates the Subversion error pool. Essentially the Subversion code
>> does this
>> 
>>   apr_pool_clear_debug (some_pool, file_line);
>>   apr_pool_create_ex(error_pool, some_pool, abort_on_pool_failure, NULL);
>> 
>> The problem is that when some_pool was originally created by
>> apr_pool_create_ex() a mutex was created which was allocated within
>> some_pool itself.  When apr_pool_clear_debug is called it free's all
>> the allocated memory including the memory used for mutex.  Thus any
>> further attempt to use some_pool, such as for the parent of the new
>> error_pool, may fail when attempting the lock some_pool's mutex. The
>> Subversion regression tests hang attempting to lock this mutex.
> 
> Correct.  I'll look into a fix somewhere tonight.

Hmmm, well, it _is_ evening now, but I don't really have the energy
to patch it up.  If someone beats me to it, fine, otherwise just don't
run with apr pool debugging enabled for now.


Sander


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:

> > From: Philip Martin
> > Sent: 29 May 2002 23:59
> 
> > "Sander Striker" <st...@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

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

Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin
> Sent: 29 May 2002 23:59

> "Sander Striker" <st...@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.
 
> [snip]
>>> This patch recreates the mutex.  That works fine in Subversion but
>>> then Subversion only has one thread!  What is this mutex for?  It's
>>> used in apr_pool_walk_tree so that threads can safely access pools
>>> used in other threads. Now consider what happens when a pool gets
>>> cleared, the pool cleanup handlers destroy the mutex and the pool
>>> free's the memory occupied by the mutex.  Nowhere does the code
>>> consider that another thread might have the mutex locked.  Nowhere
>>> does the code consider that another thread might be blocked waiting to
>>> lock the mutex.  I don't understand how this can work. Until the pool
>>> has been removed from its parent's list of children the mutex cannot
>>> be destroyed.
>> 
>> This is all correct.  I thought it was general knowledge that if you
>> go around clearing/destroying a pool in one thread and using it in the
>> other, you are asking for trouble.  I think we need to document this,
>> but I don't see it as something we should fix.
> 
> Again I don't understand.  By "using it" are you referring to the
> debug only functions? The ones mentioned in the comment above the
> mutex creation?  What is this mutex for if not to allow multiple
> threads access to the pool tree?

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.

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.

Sander

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@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.

[snip]

> > This patch recreates the mutex.  That works fine in Subversion but
> > then Subversion only has one thread!  What is this mutex for?  It's
> > used in apr_pool_walk_tree so that threads can safely access pools
> > used in other threads. Now consider what happens when a pool gets
> > cleared, the pool cleanup handlers destroy the mutex and the pool
> > free's the memory occupied by the mutex.  Nowhere does the code
> > consider that another thread might have the mutex locked.  Nowhere
> > does the code consider that another thread might be blocked waiting to
> > lock the mutex.  I don't understand how this can work. Until the pool
> > has been removed from its parent's list of children the mutex cannot
> > be destroyed.
> 
> This is all correct.  I thought it was general knowledge that if you
> go around clearing/destroying a pool in one thread and using it in the
> other, you are asking for trouble.  I think we need to document this,
> but I don't see it as something we should fix.

Again I don't understand.  By "using it" are you referring to the
debug only functions? The ones mentioned in the comment above the
mutex creation?  What is this mutex for if not to allow multiple
threads access to the pool tree?

-- 
Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin
> Sent: 29 May 2002 23:30

> "Sander Striker" <st...@apache.org> writes:
> > Ok, I like this patch somewhat better.  Could you test it please?
> 
> It works, but see later.
> 
> > 
> > Furthermore I think we have a problem in the production code aswell,
> > unless noone clears the top level pool in subversion (which would
> > cause the problem to show its ugly head).  What is happening there
> > is that the mutex is created in the top level pool.  So, if you
> > do:
> > 
> >    p = svn_pool_create(p);
> > 
> >    ... pool operations on p
> > 
> >    svn_pool_clear(p);
> > 
> >    ... pool operations on p
> >    BOOM
> 
> With my patch it doesn't go BOOM.  Look at the main function for the
> test harness, subversion/tests/svn_tests_main.c, it does
> 
>     pool = svn_pool_create (NULL);
>     for (i = 1; i < array_size; i++)
>       {
>           do_test_num(... i ... );
>           svn_pool_clear (pool);
>       }
> 
> and it works fine.  That's because the mutex gets allocated in
> global_pool which is static in apr_pools.c.  The application can never
> clear global_pool, it can't even see it.

There is a difference between debug and production.  Your patch
only addressed the debug version.

> The drawback with my simple patch is of course the unbounded memory
> use.  The advantage is that it's quick-n-dirty and works :)

...

> > Ofcourse the simple fix would be to implement svn_pool_clear as
> > somewhat more than a thin wrapper.  However, if we can avoid it
> > (by not ever clearing the top level pool), that would have my
> > preference.
> > 
> > 
> > Sander
> > 
> > Index: memory/unix/apr_pools.c
> > ===================================================================
> > RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
> > retrieving revision 1.169
> > diff -u -r1.169 apr_pools.c
> > --- memory/unix/apr_pools.c     26 May 2002 09:00:08 -0000      1.169
> > +++ memory/unix/apr_pools.c     29 May 2002 19:55:31 -0000
> > @@ -1381,6 +1381,17 @@
> >  #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */
> > 
> >      pool_clear_debug(pool, file_line);
> > +
> > +
> > +#if APR_HAS_THREADS
> > +    /* If we had our own mutex, it will have been destroyed by
> > +     * the registered cleanups.  Recreate the mutex.
> > +     */
> > +    if (pool->parent == NULL || pool->parent->mutex != pool->mutex) {
> > +        (void)apr_thread_mutex_create(&pool->mutex,
> > +                                     APR_THREAD_MUTEX_NESTED, pool);
> > +    }
> > +#endif /* APR_HAS_THREADS */
> 
> This patch recreates the mutex.  That works fine in Subversion but
> then Subversion only has one thread!  What is this mutex for?  It's
> used in apr_pool_walk_tree so that threads can safely access pools
> used in other threads. Now consider what happens when a pool gets
> cleared, the pool cleanup handlers destroy the mutex and the pool
> free's the memory occupied by the mutex.  Nowhere does the code
> consider that another thread might have the mutex locked.  Nowhere
> does the code consider that another thread might be blocked waiting to
> lock the mutex.  I don't understand how this can work. Until the pool
> has been removed from its parent's list of children the mutex cannot
> be destroyed.

This is all correct.  I thought it was general knowledge that if you
go around clearing/destroying a pool in one thread and using it in the
other, you are asking for trouble.  I think we need to document this,
but I don't see it as something we should fix.

[...]
> Another alternative (perhaps what you meant by "more than a thin
> wrapper" above?) would be to avoid having the the mutex destroyed by
> the cleanup handler, and make pool_clear_debug recognise the address
> of the mutex and not free it.  Then the mutex would persist for the
> lifetime of the pool.  I don't recommend this, it introduces nasty
> coupling between the pool and mutex implementations.

Indeed.  This is why I didn't want to go that route.

Sander


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:

> How's this for an alternative?

This is a correct algorithm.  The Subversion tests pass :)

-- 
Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin
> Sent: 29 May 2002 23:30

[...]
> If you really want to go down the destroy/recreate route I think you
> need something like
> 
>       if pool has parent
>          lock parent's mutex
>          remove pool from parent's list
>          unlock parent's mutex
>       endif
>       clear pool
>       recreate pool's mutex
>       if pool has parent
>          lock parent's mutex
>          add pool to parent's list
>          unlock parent's mutex
>       endif
> 
> That, as far as I can see, is thread-safe and doesn't have the
> potentially unbounded memory problem.  The unlock while clearing the
> pool is optional, you could hold the parent's lock for the whole
> operation.

How's this for an alternative?

Sander

Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.170
diff -u -r1.170 apr_pools.c
--- memory/unix/apr_pools.c     29 May 2002 23:02:24 -0000      1.170
+++ memory/unix/apr_pools.c     29 May 2002 23:22:41 -0000
@@ -1382,13 +1382,44 @@
 APR_DECLARE(void) apr_pool_clear_debug(apr_pool_t *pool,
                                        const char *file_line)
 {
+#if APR_HAS_THREADS
+    apr_thread_mutex_t *mutex = NULL;
+#endif
+
     apr_pool_check_integrity(pool);

 #if (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE)
     apr_pool_log_event(pool, "CLEAR", file_line, 1);
 #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */

+#if APR_HAS_THREADS
+    if (pool->parent != NULL)
+       mutex = pool->parent->mutex;
+
+    /* Lock the parent mutex before clearing so that if we have our
+     * own mutex it won't be accessed by apr_pool_walk_tree after
+     * it has been destroyed.
+     */
+    if (mutex != NULL && mutex != pool->mutex) {
+       apr_thread_mutex_lock(mutex);
+    }
+#endif
+
     pool_clear_debug(pool, file_line);
+
+#if APR_HAS_THREADS
+    /* If we had our own mutex, it will have been destroyed by
+     * the registered cleanups.  Recreate the mutex.  Unlock
+     * the mutex we obtained above.
+     */
+    if (mutex != pool->mutex) {
+        (void)apr_thread_mutex_create(&pool->mutex,
+                                      APR_THREAD_MUTEX_NESTED, pool);
+
+       if (mutex != NULL)
+           (void)apr_thread_mutex_unlock(mutex);
+    }
+#endif /* APR_HAS_THREADS */
 }

 APR_DECLARE(void) apr_pool_destroy_debug(apr_pool_t *pool,


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:

> Ok, I like this patch somewhat better.  Could you test it please?

It works, but see later.

> 
> Furthermore I think we have a problem in the production code aswell,
> unless noone clears the top level pool in subversion (which would
> cause the problem to show its ugly head).  What is happening there
> is that the mutex is created in the top level pool.  So, if you
> do:
> 
>    p = svn_pool_create(p);
> 
>    ... pool operations on p
> 
>    svn_pool_clear(p);
> 
>    ... pool operations on p
>    BOOM

With my patch it doesn't go BOOM.  Look at the main function for the
test harness, subversion/tests/svn_tests_main.c, it does

    pool = svn_pool_create (NULL);
    for (i = 1; i < array_size; i++)
      {
          do_test_num(... i ... );
          svn_pool_clear (pool);
      }

and it works fine.  That's because the mutex gets allocated in
global_pool which is static in apr_pools.c.  The application can never
clear global_pool, it can't even see it.

The drawback with my simple patch is of course the unbounded memory
use.  The advantage is that it's quick-n-dirty and works :)

> 
> Ofcourse the simple fix would be to implement svn_pool_clear as
> somewhat more than a thin wrapper.  However, if we can avoid it
> (by not ever clearing the top level pool), that would have my
> preference.
> 
> 
> Sander
> 
> Index: memory/unix/apr_pools.c
> ===================================================================
> RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
> retrieving revision 1.169
> diff -u -r1.169 apr_pools.c
> --- memory/unix/apr_pools.c     26 May 2002 09:00:08 -0000      1.169
> +++ memory/unix/apr_pools.c     29 May 2002 19:55:31 -0000
> @@ -1381,6 +1381,17 @@
>  #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */
> 
>      pool_clear_debug(pool, file_line);
> +
> +
> +#if APR_HAS_THREADS
> +    /* If we had our own mutex, it will have been destroyed by
> +     * the registered cleanups.  Recreate the mutex.
> +     */
> +    if (pool->parent == NULL || pool->parent->mutex != pool->mutex) {
> +        (void)apr_thread_mutex_create(&pool->mutex,
> +                                     APR_THREAD_MUTEX_NESTED, pool);
> +    }
> +#endif /* APR_HAS_THREADS */

This patch recreates the mutex.  That works fine in Subversion but
then Subversion only has one thread!  What is this mutex for?  It's
used in apr_pool_walk_tree so that threads can safely access pools
used in other threads. Now consider what happens when a pool gets
cleared, the pool cleanup handlers destroy the mutex and the pool
free's the memory occupied by the mutex.  Nowhere does the code
consider that another thread might have the mutex locked.  Nowhere
does the code consider that another thread might be blocked waiting to
lock the mutex.  I don't understand how this can work. Until the pool
has been removed from its parent's list of children the mutex cannot
be destroyed.

If you really want to go down the destroy/recreate route I think you
need something like

      if pool has parent
         lock parent's mutex
         remove pool from parent's list
         unlock parent's mutex
      endif
      clear pool
      recreate pool's mutex
      if pool has parent
         lock parent's mutex
         add pool to parent's list
         unlock parent's mutex
      endif

That, as far as I can see, is thread-safe and doesn't have the
potentially unbounded memory problem.  The unlock while clearing the
pool is optional, you could hold the parent's lock for the whole
operation.

>  }
> 
>  APR_DECLARE(void) apr_pool_destroy_debug(apr_pool_t *pool,

Another alternative (perhaps what you meant by "more than a thin
wrapper" above?) would be to avoid having the the mutex destroyed by
the cleanup handler, and make pool_clear_debug recognise the address
of the mutex and not free it.  Then the mutex would persist for the
lifetime of the pool.  I don't recommend this, it introduces nasty
coupling between the pool and mutex implementations.

-- 
Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Sander Striker <st...@apache.org>.
> From: Philip Martin 
> Sent: 29 May 2002 16:04

>> "Sander Striker" <st...@apache.org> writes:
>> Hmmm, well, it _is_ evening now, but I don't really have the energy
>> to patch it up.  If someone beats me to it, fine, otherwise just don't
>> run with apr pool debugging enabled for now.
> 
> I'm using a patch that puts the mutex into the parent's pool.  This
> allows APR_POOL_DEBUG to be used with Subversion.  It's not a perfect
> solution due to the potential for unbounded memory use.  It's good
> enough for me...
[...]

Ok, I like this patch somewhat better.  Could you test it please?

Furthermore I think we have a problem in the production code aswell,
unless noone clears the top level pool in subversion (which would
cause the problem to show its ugly head).  What is happening there
is that the mutex is created in the top level pool.  So, if you
do:

   p = svn_pool_create(p);

   ... pool operations on p

   svn_pool_clear(p);

   ... pool operations on p
   BOOM

Ofcourse the simple fix would be to implement svn_pool_clear as
somewhat more than a thin wrapper.  However, if we can avoid it
(by not ever clearing the top level pool), that would have my
preference.


Sander

Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvs/apr/memory/unix/apr_pools.c,v
retrieving revision 1.169
diff -u -r1.169 apr_pools.c
--- memory/unix/apr_pools.c     26 May 2002 09:00:08 -0000      1.169
+++ memory/unix/apr_pools.c     29 May 2002 19:55:31 -0000
@@ -1381,6 +1381,17 @@
 #endif /* (APR_POOL_DEBUG & APR_POOL_DEBUG_VERBOSE) */

     pool_clear_debug(pool, file_line);
+
+
+#if APR_HAS_THREADS
+    /* If we had our own mutex, it will have been destroyed by
+     * the registered cleanups.  Recreate the mutex.
+     */
+    if (pool->parent == NULL || pool->parent->mutex != pool->mutex) {
+        (void)apr_thread_mutex_create(&pool->mutex,
+                                     APR_THREAD_MUTEX_NESTED, pool);
+    }
+#endif /* APR_HAS_THREADS */
 }

 APR_DECLARE(void) apr_pool_destroy_debug(apr_pool_t *pool,


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Philip Martin <ph...@codematters.co.uk>.
"Sander Striker" <st...@apache.org> writes:

> Hmmm, well, it _is_ evening now, but I don't really have the energy
> to patch it up.  If someone beats me to it, fine, otherwise just don't
> run with apr pool debugging enabled for now.

I'm using a patch that puts the mutex into the parent's pool.  This
allows APR_POOL_DEBUG to be used with Subversion.  It's not a perfect
solution due to the potential for unbounded memory use.  It's good
enough for me...

Index: memory/unix/apr_pools.c
===================================================================
RCS file: /home/cvspublic/apr/memory/unix/apr_pools.c,v
retrieving revision 1.169
diff -u -r1.169 apr_pools.c
--- memory/unix/apr_pools.c	26 May 2002 09:00:08 -0000	1.169
+++ memory/unix/apr_pools.c	29 May 2002 13:56:07 -0000
@@ -1409,6 +1409,7 @@
 #if APR_HAS_THREADS
         if (mutex)
             apr_thread_mutex_unlock(mutex);
+        apr_thread_mutex_destroy(mutex);
 #endif /* APR_HAS_THREADS */
     }
 
@@ -1494,9 +1495,14 @@
          * the pool having no lock).  However, this might actually
          * hide problems like creating a child pool of a pool
          * belonging to another thread.
+         *
+         * PROBLEM: allocating in parent pool causes potentially unbounded
+         * memory usage.
          */
-        if ((rv = apr_thread_mutex_create(&pool->mutex,
-                APR_THREAD_MUTEX_NESTED, pool)) != APR_SUCCESS) {
+        rv = apr_thread_mutex_create(&pool->mutex,
+                                     APR_THREAD_MUTEX_NESTED,
+                                     pool->parent ? pool->parent : pool);
+        if (rv != APR_SUCCESS) {
             free(pool);
             return rv;
         }

-- 
Philip

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

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

Posted by Michael Wood <mw...@its.uct.ac.za>.
On Tue, May 28, 2002 at 12:10:43AM +0200, Sander Striker wrote:
[snip]
> Hmmm, well, it _is_ evening now, but I don't really have the
> energy to patch it up.  If someone beats me to it, fine,
> otherwise just don't run with apr pool debugging enabled for
> now.
[snip]

Damn.  You mean it's not just scissors I need to worry about?

-- 
Michael Wood <mw...@its.uct.ac.za>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org