Yes, there's definitely a race condition there.  I noticed that
too, and I thought it was intentional (i.e., that the lack of a
lock was a conscious choice to avoid a performance penalty
for apps in which pools aren't shared across threads) and
harmless (because there aren't any pools in Apache shared
by multiple threads that can do apr_palloc).  But if the
non-thread-safety is intentional, there probably should be
a prominent warning about it in the include file and documentation.

--Brian

Sander Striker wrote:

>[...]
>
>>In the current apr_palloc, the lock is only around the call to new_block.
>>I think that's reasonable; new_block is manipulating a global
>>free list, so it has to be mutex-protected.
>>
>
>This triggered me to investigate the pools code again. It seems to me that
>there is a possible race condition when two threads share the same pool.
>Examine the following piece of code that is not protected by a lock:
>
>    ...
>    new_first_avail = first_avail + size;
>
>    if (new_first_avail <= blok->h.endp) {
>        debug_verify_filled(first_avail, blok->h.endp,
>                            "[apr_palloc] Ouch!  Someone trounced past the
>end "
>                            "of their allocation!\n");
>        blok->h.first_avail = new_first_avail;
>        return (void *) first_avail;
>    }
>    ...
>
>Now in a situation with 2 threads that call apr_palloc at the same time
>(both requesting a size that fits the last block):
>
>     A                                       B
>T1   new_first_avail = first_avail + size;
>T2                                           new_first_avail = first_avail +
>size;
>
>Now, the test will succeed in both A and B, effectively returning the same
>memory twice:
>
>     if (new_first_avail <= blok->h.endp) {
>         blok->h.first_avail = new_first_avail;
>         return (void *) first_avail;
>     }
>
>Or am I missing something?
>
>
>Sander
>
>