Re: Possible race condition in pools WAS: RE: Thoughts on locks

[Brian Pane]

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