InsightSoftwareConsortium / InsightSoftwareConsortium/ITK

itkParallelSparseFieldLevelSetImageFilterTest read/write race with GetPixel() / SetPixel()

Open
#4,708 5 comments 0 reactions 0 assignees View on GitHub
type:Bug
Dominant language
C++
Stars
1.7k
Forks
748
Avg merge
1d 1h
Merged PRs (30d)
64

Description

Running the itkParallelSparseFieldLevelSetImageFilterTest test with TSan:

```
WARNING: ThreadSanitizer: data race (pid=68287)
Read of size 1 at 0x00010b524516 by thread T12:
#0 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::ThreadedUpdateActiveLayerValues(double const&, itk::SparseFieldLayer>>*, itk::SparseFieldLayer>>*, unsigned int) itkParallelSparseFieldLevelSetImageFilter.hxx:1556 (ITKLevelSetsTestDriver:arm64+0x1002f7048)
#1 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::ThreadedApplyUpdate(double const&, unsigned int) itkParallelSparseFieldLevelSetImageFilter.hxx:1374 (ITKLevelSetsTestDriver:arm64+0x1002c5bac)
#2 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::Iterate()::'lambda1'(unsigned long)::operator()(unsigned long) const itkParallelSparseFieldLevelSetImageFilter.hxx:1210 (ITKLevelSetsTestDriver:arm64+0x1002e91cc)
#3 decltype(std::declval, itk::Image>::Iterate()::'lambda1'(unsigned long)&>()(std::declval())) std::__1::__invoke[abi:ue170006], itk::Image>::Iterate()::'lambda1'(unsigned long)&, unsigned long>(itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::Iterate()::'lambda1'(unsigned long)&, unsigned long&&) invoke.h:340 (ITKLevelSetsTestDriver:arm64+0x1002e9134)


Previous write of size 1 at 0x00010b524516 by thread T3:
#0 itk::Image::SetPixel(itk::Index<3u> const&, signed char const&) itkImage.h:211 (ITKLevelSetsTestDriver:arm64+0x1000978ac)
#1 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::ThreadedUpdateActiveLayerValues(double const&, itk::SparseFieldLayer>>*, itk::SparseFieldLayer>>*, unsigned int) itkParallelSparseFieldLevelSetImageFilter.hxx:1583 (ITKLevelSetsTestDriver:arm64+0x1002f72a4)
#2 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::ThreadedApplyUpdate(double const&, unsigned int) itkParallelSparseFieldLevelSetImageFilter.hxx:1374 (ITKLevelSetsTestDriver:arm64+0x1002c5bac)
#3 itk::ParallelSparseFieldLevelSetImageFilter, itk::Image>::Iterate()::'lambda1'(unsigned long)::operator()(unsigned long) const itkParallelSparseFieldLevelSetImageFilter.hxx:1210 (ITKLevelSetsTestDriver:arm64+0x1002e91cc)

```

We see from the above that one thread is writing to 0x00010b524516 and another is simultaneously reading from the same address.

Here is the relevant code, NB the fire emojis:

```c++
else if (new_value < LOWER_ACTIVE_THRESHOLD)
{
// This index will move DOWN into a negative (inside) layer.
// First check for active layer neighbors moving in the opposite direction
flag = false;
for (unsigned int i = 0; i < Neighbor_Size; ++i)
{
// READ🔥🔥🔥🔥
if (m_StatusImage->GetPixel(centerIndex + m_NeighborList.GetNeighborhoodOffset(i)) == m_StatusActiveChangingUp)
{
flag = true;
break;
}
}
if (flag)
{
++layerIt;
continue;
}

rms_change_accumulator += itk::Math::sqr(static_cast(new_value - centerValue));
// update the value of the pixel
m_OutputImage->SetPixel(centerIndex, new_value);

// Now remove this index from the active list.
release_node = layerIt.GetPointer();
++layerIt;

m_Data[ThreadId].m_Layers[0]->Unlink(release_node);
m_Data[ThreadId].m_ZHistogram[release_node->m_Index[m_SplitAxis]] =
m_Data[ThreadId].m_ZHistogram[release_node->m_Index[m_SplitAxis]] - 1;

// now add release_node to status down list
DownList->PushFront(release_node);

// WRITE🔥🔥🔥🔥
m_StatusImage->SetPixel(centerIndex, m_StatusActiveChangingDown);
}
```

To me, this indeed looks like a buggy race. But I don't know this code at all.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.