Skip to content

BUG: Release ImageIOFactory mutex before probing candidate IOs #6831

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

Open
blowekamp wants to merge 1 commit into
base: release-5.4
Choose a base branch
from
+16 −10
Open

BUG: Release ImageIOFactory mutex before probing candidate IOs #6831

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions Modules/IO/ImageBase/src/itkImageIOFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,26 @@ ImageIOFactory::CreateImageIO(const char * path, IOFileModeEnum mode)
{
std::list<ImageIOBase::Pointer> possibleImageIO;

const std::lock_guard<std::mutex> lockGuard(createImageIOMutex);

for (auto & allobject : ObjectFactoryBase::CreateAllInstance("itkImageIOBase"))
{
auto * io = dynamic_cast<ImageIOBase *>(allobject.GetPointer());
if (io)
{
possibleImageIO.emplace_back(io);
}
else
// Lock the mutex while creating all instances of ImageIOBase, to
// ensure thread safety during intialization of third-party libraries.
const std::lock_guard<std::mutex> lockGuard(createImageIOMutex);

@greptile-apps greptile-apps Bot Sep 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

View artifacts

T-Rex Ran code and verified through T-Rex


for (auto & allobject : ObjectFactoryBase::CreateAllInstance("itkImageIOBase"))
{
std::cerr << "Error ImageIO factory did not return an ImageIOBase: " << allobject->GetNameOfClass() << std::endl;
auto * io = dynamic_cast<ImageIOBase *>(allobject.GetPointer());
if (io)
{
possibleImageIO.emplace_back(io);
}
else
{
std::cerr << "Error ImageIO factory did not return an ImageIOBase: " << allobject->GetNameOfClass()
<< std::endl;
}
}
}

for (auto & k : possibleImageIO)
{
if (mode == IOFileModeEnum::ReadMode)
Expand Down
Loading