Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

3 SLIC tests: read/write race with GetPixel() / SetPixel() #4711

Closed
seanm opened this issue Jun 5, 2024 · 2 comments
Closed

3 SLIC tests: read/write race with GetPixel() / SetPixel() #4711

seanm opened this issue Jun 5, 2024 · 2 comments
Assignees
@blowekamp
Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances

Comments

@seanm
Copy link
Contributor

seanm commented Jun 5, 2024

These 3 SLIC tests:

  • itkSLICImageFilterTest3
  • SLICFixture.Blank2DImage
  • SLICFixture.ClusterInitializationOverflow

all have pretty much the same failure when run under TSan, one example:

WARNING: ThreadSanitizer: data race (pid=90804)
  Write of size 1 at 0x00010a203f4d by thread T106:
    #0 itk::NeighborhoodAccessorFunctor<itk::Image<unsigned char, 2u>>::Set(unsigned char*, unsigned char const&) const itkNeighborhoodAccessorFunctor.h:78 (ITKSuperPixelGTestDriver:arm64+0x1000cab38)
    #1 itk::NeighborhoodIterator<itk::Image<unsigned char, 2u>, itk::ZeroFluxNeumannBoundaryCondition<itk::Image<unsigned char, 2u>, itk::Image<unsigned char, 2u>>>::SetPixel(unsigned int, unsigned char const&) itkNeighborhoodIterator.hxx:37 (ITKSuperPixelGTestDriver:arm64+0x1000c29fc)
    #2 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::RelabelConnectedRegion(itk::Index<2u> const&, unsigned int, unsigned int, std::__1::vector<itk::Index<2u>, std::__1::allocator<itk::Index<2u>>>&) itkSLICImageFilter.hxx:810 (ITKSuperPixelGTestDriver:arm64+0x1000fd8e0)
    #3 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::ThreadedConnectivity(unsigned long) itkSLICImageFilter.hxx:494 (ITKSuperPixelGTestDriver:arm64+0x1000fd370)
    #4 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)::operator()(unsigned long) const itkSLICImageFilter.hxx:680 (ITKSuperPixelGTestDriver:arm64+0x1000fc95c)
    #5 decltype(std::declval<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&>()(std::declval<unsigned long>())) std::__1::__invoke[abi:ue170006]<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long>(itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long&&) invoke.h:340 (ITKSuperPixelGTestDriver:arm64+0x1000fc8e8)
    #6 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ue170006]<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long>(itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long&&) invoke.h:415 (ITKSuperPixelGTestDriver:arm64+0x1000fc824)
    #7 std::__1::__function::__alloc_func<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long), std::__1::allocator<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)>, void (unsigned long)>::operator()[abi:ue170006](unsigned long&&) function.h:193 (ITKSuperPixelGTestDriver:arm64+0x1000fc7c0)
    #8 std::__1::__function::__func<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long), std::__1::allocator<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)>, void (unsigned long)>::operator()(unsigned long&&) function.h:364 (ITKSuperPixelGTestDriver:arm64+0x1000fa6f8)

  Previous read of size 1 at 0x00010a203f4d by thread T109:
    #0 itk::NeighborhoodAccessorFunctor<itk::Image<unsigned char, 2u>>::Get(unsigned char const*) const itkNeighborhoodAccessorFunctor.h:71 (ITKSuperPixelGTestDriver:arm64+0x1000c6020)
    #1 itk::ConstNeighborhoodIterator<itk::Image<unsigned char, 2u>, itk::ZeroFluxNeumannBoundaryCondition<itk::Image<unsigned char, 2u>, itk::Image<unsigned char, 2u>>>::GetPixel(unsigned long) const itkConstNeighborhoodIterator.h:198 (ITKSuperPixelGTestDriver:arm64+0x1000c2764)
    #2 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::RelabelConnectedRegion(itk::Index<2u> const&, unsigned int, unsigned int, std::__1::vector<itk::Index<2u>, std::__1::allocator<itk::Index<2u>>>&) itkSLICImageFilter.hxx:814 (ITKSuperPixelGTestDriver:arm64+0x1000fd95c)
    #3 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::ThreadedConnectivity(unsigned long) itkSLICImageFilter.hxx:494 (ITKSuperPixelGTestDriver:arm64+0x1000fd370)
    #4 itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)::operator()(unsigned long) const itkSLICImageFilter.hxx:680 (ITKSuperPixelGTestDriver:arm64+0x1000fc95c)
    #5 decltype(std::declval<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&>()(std::declval<unsigned long>())) std::__1::__invoke[abi:ue170006]<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long>(itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long&&) invoke.h:340 (ITKSuperPixelGTestDriver:arm64+0x1000fc8e8)
    #6 void std::__1::__invoke_void_return_wrapper<void, true>::__call[abi:ue170006]<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long>(itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)&, unsigned long&&) invoke.h:415 (ITKSuperPixelGTestDriver:arm64+0x1000fc824)
    #7 std::__1::__function::__alloc_func<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long), std::__1::allocator<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)>, void (unsigned long)>::operator()[abi:ue170006](unsigned long&&) function.h:193 (ITKSuperPixelGTestDriver:arm64+0x1000fc7c0)
    #8 std::__1::__function::__func<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long), std::__1::allocator<itk::SLICImageFilter<itk::Image<unsigned char, 2u>, itk::Image<unsigned int, 2u>, float>::GenerateData()::'lambda0'(unsigned long)>, void (unsigned long)>::operator()(unsigned long&&) function.h:364 (ITKSuperPixelGTestDriver:arm64+0x1000fa6f8)

The code snippet in question:

  size_t indexStackCount = 0;
  while (indexStackCount < indexStack.size())
  {
    const IndexType & idx = indexStack[indexStackCount++];

    markerIter.SetLocation(idx);
    labelIt.SetLocation(idx);
    for (unsigned int j = 0; j < ImageDimension; ++j)
    {
      unsigned int nIdx = center + stride[j];

      if (markerIter.GetPixel(nIdx) == 0 && labelIt.GetPixel(nIdx) == requiredLabel)
      {
        indexStack.push_back(labelIt.GetIndex(nIdx));
        markerIter.SetPixel(nIdx, 1); // WRITE 🔥🔥🔥🔥
        labelIt.SetPixel(nIdx, outputLabel); // WRITE 🔥🔥🔥🔥
      }
      nIdx = center - stride[j];
      if (markerIter.GetPixel(nIdx) == 0 && labelIt.GetPixel(nIdx) == requiredLabel) // READ 🔥🔥🔥🔥
      {
        indexStack.push_back(labelIt.GetIndex(nIdx));
        markerIter.SetPixel(nIdx, 1);
        labelIt.SetPixel(nIdx, outputLabel);
      }
    }
  }
@seanm seanm added the type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances label Jun 5, 2024
@seanm
Copy link
Contributor Author

seanm commented Jun 5, 2024

@blowekamp here's the Issue for the SLIC TSan failures.

blowekamp added a commit to blowekamp/ITK that referenced this issue Jun 17, 2024
@blowekamp
BUG: Address race condition
55b3cac 
Addresses issue InsightSoftwareConsortium#4711.

When RelabelConnectedRegion is called by ThreadedConnectivity, both
outputLabel and requeredLabel are the same value. Don't re-write the
same value.
@blowekamp blowekamp mentioned this issue Jun 17, 2024
Merged
7 tasks
blowekamp added a commit to blowekamp/ITK that referenced this issue Jun 20, 2024
@blowekamp
BUG: Address race condition in SLIC filter
47bd03c 
Addresses issue InsightSoftwareConsortium#4711.

When RelabelConnectedRegion is called by ThreadedConnectivity, both
outputLabel and requeredLabel are the same value. Don't re-write the
same label value and check the unmodifeld label image first.

Also consolidated duplicate code into a loop.
hjmjohnson pushed a commit that referenced this issue Jun 21, 2024
@blowekamp @hjmjohnson
BUG: Address race condition in SLIC filter
bbf9f17 
Addresses issue #4711.

When RelabelConnectedRegion is called by ThreadedConnectivity, both
outputLabel and requeredLabel are the same value. Don't re-write the
same label value and check the unmodifeld label image first.

Also consolidated duplicate code into a loop.
@seanm
Copy link
Contributor Author

seanm commented Jul 24, 2024

Confirmed fixed!

@seanm seanm closed this as completed Jul 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@blowekamp blowekamp

Labels
type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@seanm @blowekamp