[Insight-developers] Re: Unsafe code in ObjectMorphologyImage Filter

[Miller, James V (Research)]

I think it is a problem if multiple threads write to same pixel, regardless
of whether the same value is being written.  

I don't know if this is the real scenario, but this how I think of it: 
Consider two processors.  First processor takes a value from a register
and puts it on the data bus.  After the signals "settle", it triggers
the memory to write the "settled" signal to the memory address.

Now if processor number two puts the same value on the bus while processor
number one is triggering its value, then the "unsettled" signal may get 
put in the memory address.

So the safest approach is to never have to threads attempt to write to the
same memory location.

As for your second point: all I am saying is that two threads should not
attempt
to write to the same memory location.  The other morphology filters solve
this 
problem by turning the problem around.  Given an output pixel, they scan the
input to
determine if there is a reason to set this particular pixel.  The
ObjectMorphology
filters, on the other hand, are taking an input pixel and setting all the
output 
pixels that are affected.  





> -----Original Message-----
> From: Julien Jomier [mailto:jjomier at cs . unc . edu]
> Sent: Tuesday, August 19, 2003 11:37 AM
> To: 'Miller, James V (Research)'; 'Stephen R. Aylward'; 'Lorensen,
> William E (Research)'
> Cc: insight-developers at public . kitware . com
> Subject: RE: [Insight-developers] Re: Unsafe code in
> ObjectMorphologyImageFilter
> 
> 
> Hi Jim,
> 
> Thanks for looking at this.
> 
> > Since the Evaluate() methods touches pixels in the 
> > neighborhood of the output pixel, 
> > it may be setting the value for a pixel that it does not own. 
> > If a pixel in the 
> > neighborhood is the responsibility of another thread, you 
> > could have multiple threads writing to the same pixel.
> 
> Each thread will write the same value for this pixel, so this is not a
> problem, isn't it?
> 
> > The Evaluate() methods need to be fixed such that they limit 
> > pixel writes to the outputRegionForThread.
> 
> Then we can be wrong when doing a dilation. For instance, if 
> we have two
> threads and the object is right on the boundary of the 
> outputRegionForThread
> of the first thread. If we do the dilation without updating the second
> outputRegionForThread, i.e. setting pixels (which will not be 
> updated by the
> second thread) then the dilation is wrong.
> 
> Julien
> 
> > -----Original Message-----
> > From: insight-developers-admin at itk . org 
> > [mailto:insight-developers-admin at itk . org] On Behalf Of 
> > Miller, James V (Research)
> > Sent: Tuesday, August 19, 2003 9:55 AM
> > To: 'Stephen R. Aylward'; Lorensen, William E (Research); 
> > 'Julien Jomier'
> > Cc: insight-developers at public . kitware . com
> > Subject: RE: [Insight-developers] Re: Unsafe code in 
> > ObjectMorphologyImageFilter
> > 
> > 
> > It looks like Julien submitted code to address this problem.  
> > I looked at the solution and I am not convinced this will fix 
> > the problem. (It may move it be less
> > likely.)
> > 
> > I think the problem with the filter is in the Evaluate() 
> > methods.  The filter 
> > determines whether a given pixel in the input satisfies the 
> > appropriate conditions and 
> > then calls Evaluate().  The Evaluate() method sets (or 
> > unsets) all the appropriate pixels 
> > in the neighborhood of the output pixel. 
> > 
> > Since the Evaluate() methods touches pixels in the 
> > neighborhood of the output pixel, 
> > it may be setting the value for a pixel that it does not own. 
> >  If a pixel in the 
> > neighborhood is the responsibility of another thread, you 
> > could have multiple threads writing to the same pixel.
> > 
> > The Evaluate() methods need to be fixed such that they limit 
> > pixel writes to the outputRegionForThread.
> > 
> > 
> > 
> > 
> > > -----Original Message-----
> > > From: Stephen R. Aylward [mailto:aylward at unc . edu]
> > > Sent: Sunday, August 17, 2003 7:09 PM
> > > To: Lorensen, William E (Research)
> > > Cc: insight-developers at public . kitware . com
> > > Subject: [Insight-developers] Re: Unsafe code in 
> > > ObjectMorphologyImageFilter
> > > 
> > > 
> > > 
> > > Hi,
> > > 
> > > Julien has been working on this as well, and I took a look at
> > > it too - I 
> > > am not sure what's up since it even fails on single thread.   
> > > I'll bump 
> > > it up on my to-do list.
> > > 
> > > Thanks!
> > > 
> > > Stephen
> > > 
> > > Lorensen, William E (Research) wrote:
> > > > Steven
> > > > I noticed intermittent test failures for the subject
> > > filter. I tracked the
> > > > problem down to the ThreadedGenerateData method. There is a
> > > loop that copies
> > > > data from the input to the output region. Unfortunately,
> > > since the region
> > > > extends beyond the pixels that should be updated in a
> > > thread, some of this
> > > > pixels overlay pixels in other threads. As a temporary fix,
> > > I put a mutex
> > > > around the loop. The real fix would be to just copy the
> > > pixels that will be
> > > > affected in a given thread. Then the mutex can be removed.
> > > > 
> > > > Forget half of what I just said...
> > > > After putting in the mutex, I get different failures. I
> > > think the filter
> > > > needs to be reviewed further for thread safety. I suspect the 
> > > > IsObjectPixelOnBoundary is messing up. For now, I've set
> > > the number of
> > > > threads to 1 for this filter.
> > > > 
> > > > Bill
> > > 
> > > _______________________________________________
> > > Insight-developers mailing list
> > > Insight-developers at itk . org 
> > > http://www . itk . org/mailman/listinfo/insight-developers
> > > 
> > _______________________________________________
> > Insight-developers mailing list
> > Insight-developers at itk . org 
> > http://www . itk . org/mailman/listinfo/insight-> developers
> > 
>