[Insight-developers] LightObject race condition fix

[Steve M. Robbins]

Hi,

I'm not sure what race conditions Julia Smith has uncovered, but the
first change looks like it creates a worse race.


On Tue, Jul 10, 2007 at 08:51:21PM +0200, Gaëtan Lehmann wrote:

> Index: Code/Common/itkLightObject.cxx
> ===================================================================
> RCS file: /cvsroot/Insight/Insight/Code/Common/itkLightObject.cxx,v
> retrieving revision 1.34
> diff -u -r1.34 itkLightObject.cxx
> --- Code/Common/itkLightObject.cxx      4 Apr 2007 20:04:10 -0000       
> 1.34
> +++ Code/Common/itkLightObject.cxx      10 Jul 2007 18:41:08 -0000
> @@ -150,12 +150,12 @@
> ::UnRegister() const
> {
>    m_ReferenceCountLock.Lock();
> -  m_ReferenceCount--;
> +  int tmpReferenceCount = --m_ReferenceCount;
>    m_ReferenceCountLock.Unlock();
>
     [***]

>    // ReferenceCount in now unlocked.  We may have a race condition
>    // to delete the object.
> -  if ( m_ReferenceCount <= 0)
> +  if ( tmpReferenceCount <= 0)
>      {
>      delete this;
>      }

Imagine that thread 1 has a reference to a LightObject L with
m_ReferenceCount = 1 and calls L->Unregister().  The test for zero
references happens outside the critical section, so imagine what
happens when thread 2 calls L->Register() while thread 1 is at line
[***].  Thread 2 thinks it has obtained a reference and is going to be
very unhappy after thread 1 deletes L.

I'm not clear on the semantics of "volatile", so it's not clear to me
whether the other change (marking m_ReferenceCount as volatile) is
sufficient to cure this race in the original code.

If volatile isn't sufficient, why not just "delete this" while holding
the lock?  You'd also need to worry about threads blocking in Register()
while the object is deleted.  Presumably SimpleFastMutexLock.Lock()
would have to throw if the lock was destructed.  Or something.

My head hurts.
-Steve


-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://www.itk.org/mailman/private/insight-developers/attachments/20070710/a8c35ced/attachment.pgp