You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ruediger Pluem <rp...@apache.org> on 2020/11/25 09:20:15 UTC

Re: svn commit: r1883801 - /apr/apr/trunk/memory/unix/apr_pools.c


On 11/24/20 10:12 PM, ylavic@apache.org wrote:
> Author: ylavic
> Date: Tue Nov 24 21:12:37 2020
> New Revision: 1883801
> 
> URL: http://svn.apache.org/viewvc?rev=1883801&view=rev
> Log:
> apr_pools: follow up to r1883750 and r1883800.
> 
> After r1883800, the mutex of a pool in APR_POOL_DEBUG can't be NULL, so
> remove useless NULL checks around locking.

I am struggling a bit to see when the mutex could have been NULL before r1883800.

Regards

RĂ¼diger

Re: svn commit: r1883801 - /apr/apr/trunk/memory/unix/apr_pools.c

Posted by Ruediger Pluem <rp...@apache.org>.

On 11/25/20 11:45 AM, Yann Ylavic wrote:
> On Wed, Nov 25, 2020 at 10:20 AM Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 11/24/20 10:12 PM, ylavic@apache.org wrote:
>>> Author: ylavic
>>> Date: Tue Nov 24 21:12:37 2020
>>> New Revision: 1883801
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1883801&view=rev
>>> Log:
>>> apr_pools: follow up to r1883750 and r1883800.
>>>
>>> After r1883800, the mutex of a pool in APR_POOL_DEBUG can't be NULL, so
>>> remove useless NULL checks around locking.
>>
>> I am struggling a bit to see when the mutex could have been NULL before r1883800.
> 
> It could have been NULL because apr_pool_create_ex_debug() was adding
> the pool to the parent's children list before creating the mutex.
> In this window, apr_pool_walk_tree() (like apr_pool_find() starting
> from the global_pool) could have found the pool with its NULL mutex
> and walked it unlocked.

Ahh. Thanks for explaining. I did not consider this short period of time and that another thread doing
apr_pool_walk_tree could then hit the NULL mutex.

Regards

RĂ¼diger

Re: svn commit: r1883801 - /apr/apr/trunk/memory/unix/apr_pools.c

Posted by Yann Ylavic <yl...@gmail.com>.
On Wed, Nov 25, 2020 at 10:20 AM Ruediger Pluem <rp...@apache.org> wrote:
>
> On 11/24/20 10:12 PM, ylavic@apache.org wrote:
> > Author: ylavic
> > Date: Tue Nov 24 21:12:37 2020
> > New Revision: 1883801
> >
> > URL: http://svn.apache.org/viewvc?rev=1883801&view=rev
> > Log:
> > apr_pools: follow up to r1883750 and r1883800.
> >
> > After r1883800, the mutex of a pool in APR_POOL_DEBUG can't be NULL, so
> > remove useless NULL checks around locking.
>
> I am struggling a bit to see when the mutex could have been NULL before r1883800.

It could have been NULL because apr_pool_create_ex_debug() was adding
the pool to the parent's children list before creating the mutex.
In this window, apr_pool_walk_tree() (like apr_pool_find() starting
from the global_pool) could have found the pool with its NULL mutex
and walked it unlocked.
For a short leaving pool, the pool could then have been cleared while
a walker was accessing a child => boom!

Regards;
Yann.