Skip to content

BUG: Removing open HDF5 files causes segmentation fault#5784

Merged
hjmjohnson merged 1 commit intomainfrom
hdf5-test-file-close-test-failure
Feb 11, 2026
Merged

BUG: Removing open HDF5 files causes segmentation fault#5784
hjmjohnson merged 1 commit intomainfrom
hdf5-test-file-close-test-failure

Conversation

@hjmjohnson
Copy link
Copy Markdown
Member

Added an isolation block for the streaming readers and writers.

For streaming to work, both the ReaderImageFilter and the
WriteImageFilter will maintain handles to open HDF5 files
Until their destructors are run.

Enhanced comments for better readability and maintainability.

This failure exposed itself during debugging of PR:

PR Checklist

@github-actions github-actions bot added type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:IO Issues affecting the IO module labels Feb 9, 2026
Copy link
Copy Markdown
Member

@dzenanz dzenanz left a comment

Choose a reason for hiding this comment

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

Thank you Hans. Looks tricky.

For possible other reviewers: turn on "hide whitespace", hidden under the gear in top-right corner on GitHub.

@hjmjohnson
Copy link
Copy Markdown
Member Author

\azp ITK.macOS.Python

@hjmjohnson
Copy link
Copy Markdown
Member Author

/azp ITK.macOS.Python

@hjmjohnson hjmjohnson force-pushed the hdf5-test-file-close-test-failure branch from dc69026 to e8e7ba9 Compare February 9, 2026 21:23
Copy link
Copy Markdown
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

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

🥇

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a segfault in the HDF5 streaming read/write test by ensuring HDF5-backed pipeline objects are destroyed (and thus their file handles closed) before attempting to delete the output file.

Changes:

  • Wraps streaming writer/reader pipeline objects in an isolation scope to force destructor execution before cleanup.
  • Updates/expands comments explaining why the isolation scope is needed.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Modules/IO/HDF5/test/itkHDF5ImageIOStreamingReadWriteTest.cxx Outdated
@blowekamp blowekamp self-requested a review February 11, 2026 14:06
Copy link
Copy Markdown
Member

@blowekamp blowekamp left a comment

Choose a reason for hiding this comment

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

LGTM, Copilot suggested could be applied.

Added an isolation block for the streaming readers and writers.

For streaming to work, both the ReaderImageFilter and the
WriteImageFilter will maintain handles to open HDF5 files
Until their destructors are run.

Enhanced comments for better readability and maintainability.
@hjmjohnson hjmjohnson force-pushed the hdf5-test-file-close-test-failure branch from 99bd966 to d155450 Compare February 11, 2026 17:24
@hjmjohnson hjmjohnson merged commit b524cbc into main Feb 11, 2026
10 of 16 checks passed
@hjmjohnson hjmjohnson deleted the hdf5-test-file-close-test-failure branch February 11, 2026 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module type:Bug Inconsistencies or issues which will cause an incorrect result under some or all circumstances type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants