[Insight-developers] LightObject race condition fix

[Luis Ibanez]

Hi Gaetan,

We probably should setup a profiling test before committing
these changes.

It could be something as simple as creating and destroying
any ITK object for 10,000 times, and measuring the total
time using the itkTimesProbeCollectorBase.

That will give us an idea of the variations in computation
time, before and after introducing the changes.


   Luis


----------------------
Gaëtan Lehmann wrote:
> 
> Le 6 août 07 à 13:38, Bill Lorensen a écrit :
> 
>> Gaeten,
>>
>> Which changes did we decide on? We should be very careful here. We  
>> don't want any efficiency problems.
>>
> 
> yes, that's a sensible peace of code - that's why I made the changes  as 
> small as possible. More changes should be carrefully reviewed.
> 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
> 
> 
>> Bill
>>
>> On 8/6/07, Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr> wrote:
>> Le 12 juil. 07 à 00:31, Peter Cech a écrit :
>>
>> > On Wed, Jul 11, 2007 at 11:02:16 -0400, Karthik Krishnan wrote:
>> >> On 7/11/07, Gaëtan Lehmann <gaetan.lehmann at jouy.inra.fr> wrote:
>> >>>
>> >>>
>> >>> Le 11 juil. 07 à 15:33, Karthik Krishnan a écrit :
>> >>>
>> >>>> However I agree with Steve/Luis that access of the resource
>> >>>> m_ReferenceCount be thread safe. One solution would be to use a
>> >>>> scoped lock (unlocking is implemented in its destructor, so it
>> >>>> automatically does that when it goes out of scope).
>> >>>>
>> >>>> The code would look like
>> >>>>
>> >>>> UnRegister()
>> >>>> {
>> >>>>   ScopedLock lock( m_ReferenceCountLock )
>> >>>>   m_ReferenceCount--;
>> >>>>   if ( m_ReferenceCount <= 0) { delete this;  }
>> >>>> }
>> >>>
>> >>> Please correct if I'm wrong, but the mutex lock will be  destroyed by
>> >>> the delete call before the destruction of the ScopedLock object,
>> >>
>> >>
>> >> You're right.. How silly of me :) I'd suggest leaving it as is..
>> >
>> > It's past midnight here, so I'll refrain from trying to post any
>> > solutions, but there is something relevant here:
>> >
>> > http://softwarecommunity.intel.com/isn/Community/en-US/forums/
>> > thread/30225804.aspx
>> >
>> > Enjoy!
>> >
>>
>> Hi,
>>
>> I realize I haven't put the changes in the cvs repository.
>> Can I do that ?
>>
>> Would be great also to explore Peter's link. It seem quite complex
>> though, and I didn't get enough time yet to look at it carefully...
>>
>> Gaëtan
>>
>>
>> -- 
>> 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
>>
>>
>>
>> _______________________________________________
>> 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
> 
> 
> 
> _______________________________________________
> Insight-developers mailing list
> Insight-developers at itk.org
> http://www.itk.org/mailman/listinfo/insight-developers
>