apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sander Striker <stri...@apache.org>
Subject Re: apr_pools and thread safety
Date Wed, 29 Jun 2005 17:28:50 GMT
E Holyat wrote:

[...]
> But I can see the following still holds true for allocator_alloc and 
> allocator_free.  allocator_alloc allows a free read on max_index when 
> either allocator_alloc / allocator_free can change that value.
>
> If 40 threads from the same parent create child pools. 40 threads can 
> fall into the first "if statement". The access within that "if 
> statement" is mutually exclusive, but, if max_index is being decremented 
> within the "if statement", then the condition that put the 13 or 14th or 
> nth thread into that "if statement" may no longer hold true.  Wouldn't 
> that invalidate the index alignment check and give a thread an 
> invalid child pool reference?

I don't think this would be the case.  Comments inline.

> The mutex should be moved outside of the if and else if

> if (index <= allocator->max_index) { can be changed while being read

While allocator->max_index can indeed be changed, this is just a quick
scan to see if a node (aka chunk of memory) *might* be available.  If
this seems to be the case a mutex is taken out and the value is read
again:

apr_pools.c:196

    if (index <= allocator->max_index) {
#if APR_HAS_THREADS
        if (allocator->mutex)
            apr_thread_mutex_lock(allocator->mutex);
#endif /* APR_HAS_THREADS */

        /* Walk the free list to see if there are
         * any nodes on it of the requested size

	...

        max_index = allocator->max_index;
        ref = &allocator->free[index];
        i = index;
        while (*ref == NULL && i < max_index) {
           ref++;
           i++;
        }

At this point we either have a node or we don't.  If we do, we unlock
the mutex and return the node.  If we don't, we unlock the mutex and
fall through, and back, on this piece of code:

apr_pools.c:297

    /* If we haven't got a suitable node, malloc a new one
     * and initialize it.
     */
    if ((node = malloc(size)) == NULL)
        return NULL;

    ...

    return node;


> }
> 
> /* If we found nothing, seek the sink (at index 0), if
>  * it is not empty.
>  */
> else if (allocator->free[0]) { 2 threads can enter.

The same holds true for the above, we grab the lock and check again.
Again with the risk of falling through to the block at line 297:

apr_pools.c:260

    else if (allocator->free[0]) {
#if APR_HAS_THREADS
        if (allocator->mutex)
            apr_thread_mutex_lock(allocator->mutex);
#endif /* APR_HAS_THREADS */

        /* Walk the free list to see if there are
         * any nodes on it of the requested size
         */
        ref = &allocator->free[0];
        while ((node = *ref) != NULL && index > node->index)
            ref = &node->next;


Sander

Mime
View raw message