BUG: Release ImageIOFactory mutex before probing candidate IOs

[blowekamp]

Summary

ImageIOFactory::CreateImageIO() holds createImageIOMutex for its entire body, including the CanReadFile()/CanWriteFile() probing loop over every registered ImageIOBase. Several IO CanReadFile/CanWriteFile implementations (e.g. GDCMImageIO) open and parse the file, so this serializes every ReadImage/WriteImage call in the process — across all images, not just concurrent access to the same file.

Under concurrent reads of many distinct files from many threads, this collapses effective throughput. It can look indistinguishable from a hang once concurrent demand is high enough, since a large number of threads all queue up on this single mutex while only one thread makes progress at a time, each turn potentially doing blocking disk I/O.

This is the same underlying issue behind the intermittent hang in SimpleITK's Wrapping/Python/tests/ConcurrentImageRead.py (labeled UNSTABLE on macOS). Confirmed with lldb: at the moment of the "hang," 70 of 72 worker threads were blocked in std::mutex::lock() inside this function, while the lock holder was inside GDCMImageIO::CanReadFile()'s fopen().

Fix

Narrow the lock to only the ObjectFactoryBase::CreateAllInstance call that builds the candidate IO list; release it before probing files, so independent reads/writes on different files no longer serialize.

This mirrors the concurrency portion of b5def87 ("ENH: Two-phase ImageIOFactory dispatch via extension check", main), backported here as a minimal, behavior-preserving fix for the release-5.4 branch — without the extension-dispatch API changes from that commit, which aren't appropriate for a stable release branch.

Test plan

• Confirm this applies cleanly to release-5.4 and existing itkImageIOFactory/related tests still pass
• Optionally add a targeted concurrency regression test (a std::thread-based stress test reading many distinct files concurrently), mirroring SimpleITK's ConcurrentImageRead.py

[greptile-apps]

Greptile Summary

This change limits the ImageIO factory mutex to candidate construction, allowing CanReadFile and CanWriteFile probes for independent operations to proceed without the process-wide factory lock. The new concurrency behavior has no registered regression test, so a later lock-scope change could restore serialized probing without affecting ordinary IO behavior.

T-Rex validation blocked

A native runtime build could not be completed because the generated ITKIOImageBaseExport.h header is unavailable without a configured build, and the required CMake and Pixi tools are unavailable in this environment.

Confidence Score: 4/5

The lock-scope change should not be merged without a regression test that protects concurrent ImageIO probing.

The source and test-tree comparison directly confirms that the mutex is released before format probes and that no test was added to protect that behavior. Native execution could not be completed because the configured ITK build prerequisites are unavailable.

Files Needing Attention: Add coverage under Modules/IO/ImageBase/test for the changed behavior in Modules/IO/ImageBase/src/itkImageIOFactory.cxx.

T-Rex Logs

What T-Rex did

• T-Rex produced proofs for a posted P2 finding and linked the corresponding validation evidence.
• A coverage inspection script for the ImageIO factory probe-lock was prepared and used to validate lock behavior before and after the change.
• Pre-change and post-change probe-lock logs were captured to compare the mutex behavior surrounding the ImageIO factory changes.
• A standalone compilation attempt without a configured ITK build was performed, confirming that a native test build cannot be provisioned in this environment.
• The mutex relocation and probing strategy were described with the exact before/after lines and an attached inspection script.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. General comment

   No regression test protects unlocked ImageIO probing

   • Bug
     • CreateImageIO now intentionally releases createImageIOMutex before potentially expensive CanReadFile and CanWriteFile probes, but PR BUG: Release ImageIOFactory mutex before probing candidate IOs #6831 adds no test that makes concurrent factory calls and detects probe serialization. A future lock-scope regression would therefore pass the changed module's current test registration.
   • Cause
     • The reviewed commit changes only Modules/IO/ImageBase/src/itkImageIOFactory.cxx; no source or registration under Modules/IO/ImageBase/test was added or modified, and the registered/test-tree searches found no ImageIOFactory probe concurrency coverage.
   • Fix
     • Add a registered ImageBase regression test that registers a controllable ImageIO whose read/write probe waits or records simultaneous entry, invokes ImageIOFactory::CreateImageIO concurrently for independent paths, and fails if the probes serialize. Exercise both ReadMode and WriteMode or share the common assertion through a parameterized test.

   _{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "BUG: Release ImageIOFactory mutex before..." | Re-trigger Greptile}

[greptile-apps]

Concurrent probe coverage is missing

CreateImageIO now deliberately releases createImageIOMutex before CanReadFile and CanWriteFile, but no registered test runs independent factory calls concurrently and detects serialized probes. A future lock-scope regression could therefore restore process-wide probe serialization while ordinary IO tests still pass. Add a controllable ImageIO test double that records simultaneous read and write probe entry, then assert concurrent CreateImageIO calls for independent paths reach the probe concurrently.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Artifacts

ImageIO factory probe-lock coverage inspection script

• The executed Bash script compares the parent and reviewed revision's mutex/probe placement and ImageBase test coverage, ending with the test-tree status.

ImageIO factory probe lock before the change

• Running the inspection script against `HEAD^` shows the mutex before both read and write probe calls, with no relevant registered or source-level ImageBase test coverage.

ImageIO factory probe lock after the change

• Running the same inspection against `HEAD` shows the mutex block ends before the read/write probes and confirms the reviewed commit contains no ImageBase test-tree change.

Standalone compilation attempt without configured ITK build

• A real C++ syntax-only invocation of the changed source fails because generated `ITKIOImageBaseExport.h` is absent, confirming why the ITK runtime path could not be executed.

Working tree after evidence capture

• The captured `git status --short` records the evidence-only worktree state after validation, with no repository source files edited.

_{Ran code and verified through T-Rex}