Skip to content

Commit

Permalink
BUG: fixed concurrant writes to an ivar found by Thread Sanitizer
Browse files Browse the repository at this point in the history
Multiple threads call this method which all write to this->m_ANTSAssociate simultaneously. At least they are writing the same thing, but it's still undefined behaviour without any mutex or similar protection. Used std::call_once to ensure only one writer actually writes.
  • Loading branch information
seanm authored and hjmjohnson committed Jan 16, 2022
1 parent d5a91d1 commit ed6d2d1
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "itkConstNeighborhoodIterator.h"

#include <deque>
#include <mutex>

namespace itk
{
Expand Down Expand Up @@ -299,6 +300,7 @@ class ITK_TEMPLATE_EXPORT ANTSNeighborhoodCorrelationImageToImageMetricv4GetValu
/** Internal pointer to the metric object in use by this threader.
* This will avoid costly dynamic casting in tight loops. */
TNeighborhoodCorrelationMetric * m_ANTSAssociate;
std::once_flag m_ANTSAssociateOnceFlag;
};


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@ ANTSNeighborhoodCorrelationImageToImageMetricv4GetValueAndDerivativeThreader<TDo
const ThreadIdType threadId)
{
/* Store the casted pointer to avoid dynamic casting in tight loops. */
this->m_ANTSAssociate = dynamic_cast<TNeighborhoodCorrelationMetric *>(this->m_Associate);
if (this->m_ANTSAssociate == nullptr)
auto associate = dynamic_cast<TNeighborhoodCorrelationMetric *>(this->m_Associate);
if (associate == nullptr)
{
itkExceptionMacro("Dynamic casting of associate pointer failed.");
}

std::call_once(this->m_ANTSAssociateOnceFlag, [this, &associate]() { this->m_ANTSAssociate = associate; });

VirtualPointType virtualPoint;
MeasureType metricValueResult = NumericTraits<MeasureType>::ZeroValue();
MeasureType metricValueSum = NumericTraits<MeasureType>::ZeroValue();
Expand All @@ -50,15 +52,15 @@ ANTSNeighborhoodCorrelationImageToImageMetricv4GetValueAndDerivativeThreader<TDo
DerivativeType & localDerivativeResult = this->m_GetValueAndDerivativePerThreadVariables[threadId].LocalDerivatives;

/* Create an iterator over the virtual sub region */
// this->m_ANTSAssociate->InitializeScanning( virtualImageSubRegion, scanIt, scanMem, scanParameters );
// associate->InitializeScanning( virtualImageSubRegion, scanIt, scanMem, scanParameters );
this->InitializeScanning(virtualImageSubRegion, scanIt, scanMem, scanParameters);

/* Iterate over the sub region */
scanIt.GoToBegin();
while (!scanIt.IsAtEnd())
{
/* Get the virtual point */
this->m_ANTSAssociate->TransformVirtualIndexToPhysicalPoint(scanIt.GetIndex(), virtualPoint);
associate->TransformVirtualIndexToPhysicalPoint(scanIt.GetIndex(), virtualPoint);

/* Call the user method in derived classes to do the specific
* calculations for value and derivative. */
Expand Down

0 comments on commit ed6d2d1

Please sign in to comment.