[Insight-developers] LightObject race condition fix

[Gaëtan Lehmann]

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