[Insight-developers] Re: LightObject race condition fix

[Gaëtan Lehmann]

Le 6 août 07 à 18:29, Simon Warfield a écrit :

>
> So imagine the following sequence of events:
> Object A in thread 1 calls UnRegister() on a LightObject L shared  
> across threads.
>
> L obtains a lock, and decrements the temporary reference count to  
> be equal to zero, and releases the lock.
>
> Object B in thread 2 , unaware that Object A has asked L to destroy  
> itself, calls Register().
> It obtains a lock on the reference count variable, and  
> m_ReferenceCount variable goes back up to 1, and then the lock is  
> released.
>
> Light object L then commits suicide by calling delete this, since  
> its temporary reference count variable is 0.
>
> Object B is not expecting this to happen, since it just incremented  
> the reference count.
> Object B dereferences some pointer  inside L (or a class derived  
> from L) was managing for it, gets random memory contents, and  
> crashes with a segmentation violation.

Yes, that's a problem, but it is also there currently.
This patch does not try to fix that - only the problem reported by  
Julia Smith.

Gaëtan


>> The changes are:
>>
>> [glehmann at marvin Common]$ cvs diff -u
>> Index: itkLightObject.cxx
>> ===================================================================
>> RCS file: /cvsroot/Insight/Insight/Code/Common/itkLightObject.cxx,v
>> retrieving revision 1.34
>> diff -u -r1.34 itkLightObject.cxx
>> --- itkLightObject.cxx  4 Apr 2007 20:04:10 -0000       1.34
>> +++ itkLightObject.cxx  6 Aug 2007 12:07:57 -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;
>>       }
>> Index: itkLightObject.h
>> ===================================================================
>> RCS file: /cvsroot/Insight/Insight/Code/Common/itkLightObject.h,v
>> retrieving revision 1.33
>> diff -u -r1.33 itkLightObject.h
>> --- itkLightObject.h    6 Feb 2006 22:01:56 -0000       1.33
>> +++ itkLightObject.h    6 Aug 2007 12:07:57 -0000
>> @@ -114,7 +114,7 @@
>>     virtual void PrintTrailer(std::ostream& os, Indent indent) const;
>>
>>     /** Number of uses of this object by other objects. */
>> -  mutable int m_ReferenceCount;
>> +  volatile mutable int m_ReferenceCount;
>>     /** Mutex lock to protect modification to the reference count */
>>     mutable SimpleFastMutexLock m_ReferenceCountLock;
>>
>> Gaëtan
>>
>>
>
> -- 
> Simon
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers

--
Gaëtan Lehmann
Biologie du Développement et de la Reproduction
INRA de Jouy-en-Josas (France)
tel: +33 1 34 65 29 66    fax: 01 34 65 29 09
http://voxel.jouy.inra.fr