BUG: C17: Concurrency and non-terminating loops in 11 filters #1876

Summary

Some filters share mutable state between threads with no synchronization, or run a loop that can continue forever and does not check for cancel. A data race can corrupt memory and crash DREAM3D-NX or NXRunner. A loop that never ends hangs the application, and the user cannot cancel it.

As of develop 419f9db (2026-10-21) this class has 12 open findings in 11 filters: 12 CRASH, 0 WRONG_DATA, 0 MINOR. An automated code-reading sweep found them on develop 4a7b71e5f (2026-10-08). Each finding was re-checked by reading the code on develop 419f9db. Each finding is a candidate: nobody has run it yet.

Mechanism

This class has two sub-patterns.

1. Shared mutable state with no synchronization. Several filters keep a global std::map s_HeaderCache keyed by an instance id. The constructor inserts, the destructor erases, and preflight writes, with no mutex. For example CropImageGeometryFilter.cpp#L65-L75 (the map is declared at line 42):

CropImageGeometryFilter::CropImageGeometryFilter()
: m_InstanceId(s_InstanceId.fetch_add(1))
{
  s_HeaderCache[m_InstanceId] = {};
}

//------------------------------------------------------------------------------
CropImageGeometryFilter::~CropImageGeometryFilter() noexcept
{
  s_HeaderCache.erase(m_InstanceId);
}

The GUI preflights filters on one thread while a pipeline runs on another thread. Two threads that insert into or erase from the same std::map at the same time cause undefined behavior. The correct form already exists on develop in ReadNIfTIFileFilter.cpp#L54-L74 (from #1595), ReadZeissTxmFileFilter.cpp and ReadOnScaleTableFileFilter.cpp (from #1741). They lock s_HeaderCacheMutex around every access:

std::mutex s_HeaderCacheMutex;
...
ReadNIfTIFileFilter::ReadNIfTIFileFilter()
: m_InstanceId(s_InstanceId.fetch_add(1))
{
  std::lock_guard<std::mutex> lock(s_HeaderCacheMutex);
  s_HeaderCache[m_InstanceId] = {};
}

The same sub-pattern includes parallel workers that call operator[] on a shared std::map (this inserts a key), and ParallelTaskAlgorithm / ParallelDataAlgorithm users that write to out-of-core stores without calling requireArraysInMemory / requireStoresInMemory (IParallelAlgorithm.hpp#L35-L37). Out-of-core stores do not support concurrent access.

2. A loop with no progress guarantee and no cancel check. The loop ends only when the data allows it, and the data is not validated. Examples: an exhaustive search over all threshold combinations, an iterate-until-no-change loop that counts cells that did not change (ErodeDilateCoordinationNumber.cpp#L211), rejection sampling when no candidate is valid, and a blocking wait on a child process with no timeout. These loops do not check shouldCancel / getCancel(), so the user cannot stop them.

No filter of this class is fixed yet.

Rule

Decision needed: no rule for this class is decided yet. Proposed rule:

• Every global or static cache that more than one filter instance can use must be protected by a mutex, as in ReadNIfTIFileFilter. A better option is to remove the global cache: store the data in the filter instance, or compute it again in execute. Decide which option to use; then use it in all filters of this class.
• Parallel workers must not change shared containers. Use find() on a const map.
• A filter that writes to shared DataArrays from parallel tasks must call requireArraysInMemory or requireStoresInMemory for those arrays.
• Every loop that can run for a long time must check shouldCancel (or getCancel()) and must have a progress guarantee: count real changes, cap the number of iterations, or use a timeout.

How to fix

1. Header caches (ReadBinaryCTNorthstarFilter, ReadVolumeGraphicsFileFilter, CropImageGeometryFilter, ReadCSVFileFilter, SplitDataArrayByTupleFilter): add a std::mutex next to s_HeaderCache and take a std::lock_guard in the constructor, the destructor, preflight and execute, as in ReadNIfTIFileFilter.cpp. Or remove the cache if the data can be computed again in execute. Consider one shared mutex-protected cache helper so that the code is not copied again.
2. Shared map in a parallel worker (SharedFeatureFaceFilter): replace m_FaceSizeMap[faceId64] with find() on a const map, and use 0 when the key is absent.
3. Out-of-core parallel writes (CombineStlFilesFilter): call taskRunner.requireArraysInMemory(...) or requireStoresInMemory(...) with the output arrays before execute.
4. Loops:
   • ErodeDilateCoordinationNumberFilter: count only cells that really change, stop when a pass makes no change, and check cancel in each pass. If Feature Ids is in Attribute Arrays to Ignore with Loop Until Gone on, consider a preflight error.
   • PointSampleTriangleGeometryFilter: before the loop, check that at least one masked Face has an area greater than 0, and return an error if none has. Check cancel in the loop.
   • OtsuMultipleThresholdsImageFilter: pass the cancel flag into OtsuMultipleThresholds and check it in the search loop. Cap the number of thresholds in preflight, or use a dynamic-programming multi-Otsu search.
   • ExecuteProcessFilter: apply Timeout to the wait (for example options.deadline, or wait(timeout) then stop()). On cancel, terminate the child process before drain_async.get(), and take the mutex for the final flush. Reject a negative Timeout in preflight.
5. New preflight errors use the error-code range of that filter. Codes are more negative than -10000, and each filter uses one range of 100 codes. Check uniqueness with git grep -nE -- "-NNNN([^0-9]|$)" on develop and on open PR refs.
6. Remove any execute-time check that a new preflight check makes unreachable.

How to test

• Write the failing test first.
• Data races: A unit test cannot show a race reliably. Write a test that constructs, preflights and destroys many instances of the filter from several std::threads at the same time. Run it under ThreadSanitizer (-fsanitize=thread) to show the race before the fix and no race after the fix. Record the sanitizer output in the PR. For CombineStlFilesFilter, run the existing test in the out-of-core build.
• Loops: Write a test with the trigger input from the checklist. Run the filter with a cancel flag that is set after a short time, or with a test timeout, and check that execute returns (with an error or a warning) instead of hanging. For ErodeDilateCoordinationNumberFilter and PointSampleTriangleGeometryFilter, also check the output values against an independent expected result for a small input.
• ExecuteProcessFilter: Run a command that does not exit (for example sleep 100) with Timeout = 1000, and check that execute returns within a few seconds.

Conventions

• Make one PR for the whole class. Make one commit for each filter.
• Start each commit message with BUG:. Add a Signed-off-by line. Do not add AI attribution.
• Start every fix with a unit test that fails on develop. If you cannot reproduce a finding, write that in the PR and on this issue with the reason. Do not force a fix.
• Value checks need an independent expected result (oracle). Do not use the output of the code under test as the expected result.
• Run ctest serially. Do not use -j, because the tests depend on each other.
• Format with clang-format 19.1.1 and the repository .clang-format.
• Do not add sentences to the filter documentation about input requirements. The generated parameter table documents them.
• If a fix touches a filter that has a V&V report (src/Plugins/<Plugin>/vv/<Filter>.md), update every file of the V&V package of that filter (report, deviations, provenance, test tables). V&V documents reference DREAM3D 6.5.171 only.
• Rebase on develop. Do not merge develop into the branch.

Checklist

• CRASH · CombineStlFilesFilter · CombineStlFiles.cpp:234 — Combine two or more STL files in an out-of-core build. ParallelTaskAlgorithm tasks copy into the shared output DataArrays, but the code does not call requireStoresInMemory, so out-of-core stores get concurrent writes. CombineStlFilesFilter#3
• CRASH · CropImageGeometryFilter · CropImageGeometryFilter.cpp:42 — Construct, destroy or preflight two instances on different threads at the same time. The global s_HeaderCache map is changed in the constructor, destructor and preflight with no mutex. CropImageGeometryFilter#8
• CRASH · ErodeDilateCoordinationNumberFilter · ErodeDilateCoordinationNumber.cpp:211 — Set Loop Until Gone = true and put the Feature Ids path in Attribute Arrays to Ignore. The counter still counts cells, but Feature Ids never change, so the loop never ends and does not check cancel. ErodeDilateCoordinationNumberFilter#1
• CRASH · ExecuteProcessFilter · ExecuteProcess.cpp:73 — Set Should Block = true, Timeout = 1000 and a command that never exits (for example sleep 100000). Timeout is used only in the stop actions; drain and process.wait(reproc::infinite) wait until the child exits, so execute hangs. ExecuteProcessFilter#2
• CRASH · ExecuteProcessFilter · ExecuteProcess.cpp:118 — Set Should Block = false, run a long command and cancel the pipeline. After cancel the final outFile << output runs without the mutex while the drain thread can still write, and drain_async.get() and wait(infinite) block until the child exits. ExecuteProcessFilter#3
• CRASH · OtsuMultipleThresholdsImageFilter · HistogramEngine.hpp:558 — Set Number Of Thresholds = 10 with the default 128 bins. OtsuMultipleThresholds tries every combination (C(bins-1, k)) and takes no cancel flag, so execute runs for a very long time and cannot be cancelled. OtsuMultipleThresholdsImageFilter#1
• CRASH · PointSampleTriangleGeometryFilter · PointSampleTriangleGeometry.cpp:131 — Set Use Mask Array = true with a mask that is false on all Faces (or true only on Faces with area 0). The while(true) rejection loop never finds a valid Face, has no limit and does not check cancel. PointSampleTriangleGeometryFilter#2
• CRASH · ReadBinaryCTNorthstarFilter · ReadBinaryCTNorthstarFilter.cpp:34 — Construct, destroy or preflight two instances on different threads (for example GUI preflight while a pipeline runs). The global s_HeaderCache map is changed and read with no mutex, so the map can be corrupted. ReadBinaryCTNorthstarFilter#5
• CRASH · ReadCSVFileFilter · ReadCSVFileFilter.cpp:43 — Preflight one instance on a worker thread while another instance is constructed or destroyed on the UI thread. The global s_HeaderCache map is changed with operator[]/erase with no mutex. ReadCSVFileFilter#5
• CRASH · ReadVolumeGraphicsFileFilter · ReadVolumeGraphicsFileFilter.cpp:337 — Construct, destroy or preflight two instances on different threads. The global s_HeaderCache map is changed in the constructor, destructor and preflight, and read in execute, with no mutex. ReadVolumeGraphicsFileFilter#5
• CRASH · SharedFeatureFaceFilter · SharedFeatureFace.cpp:48 — Use any Triangle Geometry with enough Feature faces for a parallel split. The parallel worker reads m_FaceSizeMap[faceId64], which inserts a new key into the shared std::map while other threads read it. (related: BUG: Avoid concurrent map insertion in Compute Triangle Face Ids #1772) SharedFeatureFaceFilter#3
• CRASH · SplitDataArrayByTupleFilter · SplitDataArrayByTupleFilter.cpp:39 — Construct, destroy, preflight or execute two instances on different threads. The global s_HeaderCache map is changed in the constructor, destructor and preflight, and read in execute, with no mutex. SplitDataArrayByTupleFilter#5

Related issues

• BUG: Avoid concurrent map insertion in Compute Triangle Face Ids #1772 BUG: Avoid concurrent map insertion in Compute Triangle Face Ids (same as SharedFeatureFaceFilter#3). Close it from the PR that fixes this finding.
• PR BUG: Fix OOC Review Findings #1764 ("BUG: Fix OOC Review Findings") uses find() only in its new out-of-core path of SharedFeatureFace. The in-core path is not changed there.

Out of scope

• Findings in filters that have a V&V report are not in this issue. 1 finding is excluded, in ReplaceElementAttributesWithNeighborValuesFilter. A separate V&V branch handles it.