apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <bp...@pacbell.net>
Subject Re: SMS lock on demand WAS: RE: Possible race condition...
Date Mon, 02 Jul 2001 20:08:30 GMT
Sander Striker wrote:


>Coupling the registration to thread creation, rather than SMS creation,
>resolves some of the concerns that I listed previously.  But I still have
>some questions about what happens during alloc/free in this model.
>Is your plan that the alloc/free functions should check sms->thread_count
>and do locking if it is greater than 1?
>No, when the thread_count is increased, the lock is automatically
>created in the apr_sms_thread_register function.
>In the code we do this:
>    if (SMS_XXX_T(sms)->lock)
>        apr_lock_acquire(SMS_XXX_T(sms)->lock);
I still count two, or maybe three, race conditions here:
  * The lock can become non-null between the if-statement and
    the allocation code that follows (because somebody registers
    with the SMS from another thread).
  * In apr_sms_thread_register, two threads can collide in this
        if (!sms->lock) {
            sms_lock = apr_lock_create();
    It's possible for a newly created lock to be leaked in this code.
  * I'm not sure if the pointer assignment "sms_lock = ..."
    is going to be atomic on all the supported architectures.


>With regard to the addition of parent_sms as an arg to apr_thread_create,
>am I right in assuming that NULL is valid value (meaning "don't create
>a stub and don't incur any locking overhead")?
>No. You need an sms to create your thread structure from. This is going to
>be the parent (ofcourse, this still assumes that pools are going away).
>In the thread there will be no locking (since there is only one registered
>thread). If the threads sms needs to fall back on its parent, there will
>be locking involved. To keep this down to a minimum I'm developing the
>threads sms. It'll be here real soon now(tm).
>The stub is needed for the thread registration, so it should not be
Makes sense to me.


View raw message