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
   part:
       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
avoided.

Makes sense to me.

Thanks,
--Brian





Reply via email to