From b32dbad7365db752b4b2d19e63a38a79654efcf8 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:08:09 -0700 Subject: [PATCH 1/9] Avoid Zarr-to-HDF5 export deadlock and make the IO finalizer non-blocking Read a zarr array into memory in HDF5IO.__list_fill__/__scalar_fill__ before the HDF5 write, so the read does not run while h5py's global lock is held. zarr v3 dispatches reads to a background event-loop thread; reading under that lock can deadlock against a garbage-collection finalizer that acquires the same lock on that thread. Make HDMFIO.__del__ non-blocking: a finalizer that runs a lock-acquiring close on an unpredictable thread can deadlock or hit interpreter-shutdown ordering issues. __del__ now emits a ResourceWarning for an unclosed IO (as CPython does for files), and still-open IOs are flushed and closed by an atexit handler on the main thread. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/hdmf/backends/hdf5/h5tools.py | 10 +++++ src/hdmf/backends/io.py | 39 +++++++++++++++++- tests/unit/test_io_hdf5_h5tools.py | 63 ++++++++++++++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) diff --git a/src/hdmf/backends/hdf5/h5tools.py b/src/hdmf/backends/hdf5/h5tools.py index bd2a3eaa9..de7169e21 100644 --- a/src/hdmf/backends/hdf5/h5tools.py +++ b/src/hdmf/backends/hdf5/h5tools.py @@ -1299,6 +1299,10 @@ def __scalar_fill__(cls, parent, name, data, options=None): except Exception as exc: msg = 'cannot add %s to %s - could not determine type' % (name, parent.name) raise Exception(msg) from exc + # Read a zarr array into memory before creating the dataset so its read does not + # run while the HDF5 lock is held (see __list_fill__ for the deadlock this avoids). + if is_zarr_array(data): + data = np.asarray(data) try: dset = parent.create_dataset(name, data=data, shape=None, dtype=dtype, **io_settings) except Exception as exc: @@ -1447,6 +1451,12 @@ def __list_fill__(cls, parent, name, data, options=None): new_shape = list(dset.shape) new_shape[0] = _get_length(data) dset.resize(new_shape) + # Read a zarr array into memory before the assignment so its read does not run + # while the HDF5 lock is held. zarr v3 dispatches reads to a background + # event-loop thread; reading under the HDF5 lock can deadlock against a garbage + # collection finalizer that acquires the same lock on that thread. + if is_zarr_array(data): + data = np.asarray(data) try: dset[:] = data except Exception as e: diff --git a/src/hdmf/backends/io.py b/src/hdmf/backends/io.py index 23319628f..ec1f49129 100644 --- a/src/hdmf/backends/io.py +++ b/src/hdmf/backends/io.py @@ -1,5 +1,7 @@ from abc import ABCMeta, abstractmethod +import atexit import os +import weakref from pathlib import Path from ..build import BuildManager, GroupBuilder, TypeMap @@ -10,6 +12,23 @@ from warnings import warn +# Track open HDMFIO instances so their files can be flushed and closed at interpreter +# exit if the user forgot to close them. Weak references let instances be garbage +# collected normally; close() is idempotent, so closing an already-closed instance is a +# no-op. Cleanup runs here on the main thread at a controlled time, where a blocking +# close cannot deadlock. +_open_ios = weakref.WeakSet() + + +@atexit.register +def _close_open_ios(): + for io in list(_open_ios): + try: + io.close() + except Exception: # pragma: no cover - best-effort cleanup at interpreter exit + pass + + class HDMFIO(metaclass=ABCMeta): @docval({'name': 'manager', 'type': BuildManager, @@ -33,6 +52,7 @@ def __init__(self, **kwargs): self.herd_path = herd_path self.herd = None self.open() + _open_ios.add(self) @property def manager(self): @@ -241,4 +261,21 @@ def __exit__(self, type, value, traceback): self.close() def __del__(self): - self.close() + # Do not close here. close() can acquire a lock (e.g. h5py's global lock), and + # finalizers run at unpredictable times on unpredictable threads (a background + # event-loop thread, or the main thread mid garbage collection), where a + # blocking, lock-acquiring close can deadlock. Surface the unclosed resource the + # way CPython does for files; _close_open_ios() and the backend's own file + # finalizer release the handle. + is_open = getattr(self, "is_open", None) + try: + still_open = bool(is_open()) if callable(is_open) else False + except Exception: + still_open = False + if still_open: + warn( + f"{type(self).__name__} was not closed before being garbage collected. " + "Use a context manager or call close() to release resources. " + f"Source: {self.source!r}", + ResourceWarning, + ) diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index c75a6eb82..e214ada4c 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -1,4 +1,5 @@ """Test module to validate that HDF5IO is working""" +import gc import os import unittest import warnings @@ -3762,6 +3763,22 @@ def test_roundtrip_basic(self): self.assertListEqual(foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist(), read_foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist()) + def test_write_zarr_v3_dataset_materialized(self): + # Regression: a zarr array is read into memory before the HDF5 write, so the read + # does not run while the HDF5 lock is held (see HDF5IO.__list_fill__). Uses the + # zarr v3 API (zarr.create_array) directly. + base_data = np.arange(50).astype('int32') + store = os.path.join(self.zarr_path, 'arr.zarr') + z = zarr.create_array(store=store, shape=base_data.shape, chunks=(8,), dtype='int32') + z[:] = base_data + with HDF5IO(self.path, mode='a') as io: + f = io._file + io.write_dataset(f, DatasetBuilder(name='test_dataset', data=z, attributes={})) + dset = f['test_dataset'] + self.assertTupleEqual(tuple(dset.shape), base_data.shape) + self.assertEqual(dset.dtype, base_data.dtype) + self.assertListEqual(dset[:].tolist(), base_data.tolist()) + def test_roundtrip_empty_dataset(self): zarr.save(self.zarr_path, np.asarray([]).astype('int64')) zarr_data = zarr.open(self.zarr_path, 'r') @@ -4516,3 +4533,49 @@ def test_subclasses_are_expandable(self): # VectorData subclass (custom_col) should be expandable self.assertEqual(f['custom_col'].maxshape, (None,)) self.assertIsNotNone(f['custom_col'].chunks) + + +class TestHDMFIOFinalizer(TestCase): + """The IO finalizer surfaces an unclosed IO without a blocking close. + + close() can acquire a lock that a garbage-collection finalizer must not block on, so + __del__ warns instead of closing. Cleanup of a still-open IO is deferred to an atexit + handler that runs on the main thread. + """ + + def setUp(self): + self.manager = get_foo_buildmanager() + self.path = get_temp_filepath() + + def tearDown(self): + if os.path.exists(self.path): + os.remove(self.path) + + def test_del_warns_when_still_open(self): + io = HDF5IO(self.path, manager=self.manager, mode='w') + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter("always") + del io + gc.collect() + resource_warnings = [w for w in recorded if issubclass(w.category, ResourceWarning)] + self.assertEqual(len(resource_warnings), 1) + self.assertIn("was not closed", str(resource_warnings[0].message)) + + def test_del_does_not_warn_after_close(self): + io = HDF5IO(self.path, manager=self.manager, mode='w') + io.close() + with warnings.catch_warnings(record=True) as recorded: + warnings.simplefilter("always") + del io + gc.collect() + resource_warnings = [w for w in recorded if issubclass(w.category, ResourceWarning)] + self.assertEqual(len(resource_warnings), 0) + + def test_open_io_registered_for_atexit_cleanup(self): + from hdmf.backends.io import _open_ios, _close_open_ios + + io = HDF5IO(self.path, manager=self.manager, mode='w') + self.assertIn(io, _open_ios) + self.assertTrue(io.is_open()) + _close_open_ios() # simulate interpreter-exit cleanup + self.assertFalse(io.is_open()) From b71b6101cc2da9a2ec498636aaaeb1d445106ff5 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:09:16 -0700 Subject: [PATCH 2/9] Add CHANGELOG entries for the Zarr export deadlock and finalizer fixes Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d5626cad..23224765b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,8 +9,10 @@ - Added `hdmf.build.ObjectMapper.NO_OVERRIDE`, a sentinel that a `constructor_arg` or `object_attr` override function returns to fall through to the value built from the file or read from the container. Returning `None` from an override function to signal "no override" is deprecated: it still falls through but now emits a `DeprecationWarning`. In HDMF 8.0, a `None` return will set the constructor argument or attribute to `None`. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167) - Added support for hdmf-common schema 1.10.0, which changes ``MeaningsTable.target`` from a link to an object-reference attribute (``dtype`` with ``reftype: object``, ``target_type: VectorData``). Files written with the hdmf-common 1.9.0 ``MeaningsTable`` (a link named "target") are still read correctly via a backwards-compatibility mapping in ``MeaningsTableMap``. There is no change in the read/write API; the change is limited to the representation on disk. @rly [#1525](https://github.com/hdmf-dev/hdmf/pull/1525) - ``DynamicTable.get_meanings_for_column`` (and the ``AlignedDynamicTable`` override) now returns ``None`` when the named column exists but has no ``MeaningsTable``, and raises ``KeyError`` only when the column itself does not exist. @rly [#1538](https://github.com/hdmf-dev/hdmf/pull/1538) +- `HDMFIO.__del__` emits a `ResourceWarning` for an IO that was not closed instead of calling `close()`. A finalizer runs at an unpredictable time on an unpredictable thread, where a blocking, lock-acquiring close can deadlock. Still-open IOs are flushed and closed by an `atexit` handler on the main thread. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) ### Fixed +- Fixed a deadlock when exporting a Zarr file to HDF5 with `HDF5IO.export`. A zarr array is now read into memory before the HDF5 write (in `__list_fill__`/`__scalar_fill__`), so the read does not run while h5py's global lock is held. zarr v3 dispatches reads to a background event-loop thread; reading under that lock could deadlock against a garbage-collection finalizer that acquires the same lock on that thread. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) - Fixed subclassing a `MultiContainerInterface` type without redefining `__init__`. A subclass now inherits the ancestor's constructor (custom or auto-generated) instead of regenerating one that drops the parent's docval arguments. This restores the common "generate with `get_class`, subclass to customize one method, re-register with `@register_class`" extension idiom for `DynamicTable`-family subtypes. @rly [#1540](https://github.com/hdmf-dev/hdmf/pull/1540) - Fixed `ObjectMapper._parse_isoformat` raising `ValueError: Invalid isoformat string` when reading an ISO 8601 timestamp that ends in the `Z` (UTC) designator on Python < 3.11. `datetime.fromisoformat` did not accept a trailing `Z` until Python 3.11, but `requires-python` is `>=3.10` and hdmf's own writer emits the equivalent `+00:00` offset, so a `Z`-terminated timestamp written by a peer tool was unreadable on the supported 3.10 floor. A trailing `Z`/`z` is now normalized to `+00:00` before parsing. @Leonard013 [#1539](https://github.com/hdmf-dev/hdmf/pull/1539) - Fixed reading the resolved values of an `EnumData` column from an HDF5 file (e.g. `column[:]`). @rly [#1534](https://github.com/hdmf-dev/hdmf/pull/1534) From b66acbdfa144f12f1359d847838b67a6f7fe6035 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:10:51 -0700 Subject: [PATCH 3/9] Skip the zarr v3 materialize regression test when zarr<3 is installed Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_io_hdf5_h5tools.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index e214ada4c..56f088565 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -3763,10 +3763,11 @@ def test_roundtrip_basic(self): self.assertListEqual(foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist(), read_foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist()) + @unittest.skipUnless(int(zarr.__version__.split(".")[0]) >= 3, "requires zarr v3") def test_write_zarr_v3_dataset_materialized(self): # Regression: a zarr array is read into memory before the HDF5 write, so the read - # does not run while the HDF5 lock is held (see HDF5IO.__list_fill__). Uses the - # zarr v3 API (zarr.create_array) directly. + # does not run while the HDF5 lock is held (see HDF5IO.__list_fill__). The deadlock + # this avoids is specific to zarr v3, which reads on a background thread. base_data = np.arange(50).astype('int32') store = os.path.join(self.zarr_path, 'arr.zarr') z = zarr.create_array(store=store, shape=base_data.shape, chunks=(8,), dtype='int32') From 61a620808525a5628067c4d3fd006755fa2e59d8 Mon Sep 17 00:00:00 2001 From: Ryan Ly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:21:35 -0700 Subject: [PATCH 4/9] Clean up comments --- src/hdmf/backends/io.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/hdmf/backends/io.py b/src/hdmf/backends/io.py index ec1f49129..14c5489bd 100644 --- a/src/hdmf/backends/io.py +++ b/src/hdmf/backends/io.py @@ -13,10 +13,8 @@ # Track open HDMFIO instances so their files can be flushed and closed at interpreter -# exit if the user forgot to close them. Weak references let instances be garbage -# collected normally; close() is idempotent, so closing an already-closed instance is a -# no-op. Cleanup runs here on the main thread at a controlled time, where a blocking -# close cannot deadlock. +# exit if the user forgot to close them. Cleanup runs here on the main thread at a +# controlled time, where a blocking close cannot deadlock. _open_ios = weakref.WeakSet() From bbdcab2503c9c5f905cf0852a2d45b5f702bc46b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 19:33:07 +0000 Subject: [PATCH 5/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/hdmf/backends/io.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hdmf/backends/io.py b/src/hdmf/backends/io.py index 14c5489bd..13c1bb58a 100644 --- a/src/hdmf/backends/io.py +++ b/src/hdmf/backends/io.py @@ -13,7 +13,7 @@ # Track open HDMFIO instances so their files can be flushed and closed at interpreter -# exit if the user forgot to close them. Cleanup runs here on the main thread at a +# exit if the user forgot to close them. Cleanup runs here on the main thread at a # controlled time, where a blocking close cannot deadlock. _open_ios = weakref.WeakSet() From d347810623bbdd4feb3634a81421c16913762166 Mon Sep 17 00:00:00 2001 From: Ryan Ly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:33:53 -0700 Subject: [PATCH 6/9] Clean up changelog entries --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4aacb5e18..1841b472b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,10 +9,10 @@ - Added `hdmf.build.ObjectMapper.NO_OVERRIDE`, a sentinel that a `constructor_arg` or `object_attr` override function returns to fall through to the value built from the file or read from the container. Returning `None` from an override function to signal "no override" is deprecated: it still falls through but now emits a `DeprecationWarning`. In HDMF 8.0, a `None` return will set the constructor argument or attribute to `None`. @rly [#1167](https://github.com/hdmf-dev/hdmf/pull/1167) - Added support for hdmf-common schema 1.10.0, which changes ``MeaningsTable.target`` from a link to an object-reference attribute (``dtype`` with ``reftype: object``, ``target_type: VectorData``). Files written with the hdmf-common 1.9.0 ``MeaningsTable`` (a link named "target") are still read correctly via a backwards-compatibility mapping in ``MeaningsTableMap``. There is no change in the read/write API; the change is limited to the representation on disk. @rly [#1525](https://github.com/hdmf-dev/hdmf/pull/1525) - ``DynamicTable.get_meanings_for_column`` (and the ``AlignedDynamicTable`` override) now returns ``None`` when the named column exists but has no ``MeaningsTable``, and raises ``KeyError`` only when the column itself does not exist. @rly [#1538](https://github.com/hdmf-dev/hdmf/pull/1538) -- `HDMFIO.__del__` emits a `ResourceWarning` for an IO that was not closed instead of calling `close()`. A finalizer runs at an unpredictable time on an unpredictable thread, where a blocking, lock-acquiring close can deadlock. Still-open IOs are flushed and closed by an `atexit` handler on the main thread. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) +- `HDMFIO.__del__` emits a `ResourceWarning` for an IO that was not closed instead of calling `close()`. Still-open IOs are flushed and closed by an `atexit` handler on the main thread. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) ### Fixed -- Fixed a deadlock when exporting a Zarr file to HDF5 with `HDF5IO.export`. A zarr array is now read into memory before the HDF5 write (in `__list_fill__`/`__scalar_fill__`), so the read does not run while h5py's global lock is held. zarr v3 dispatches reads to a background event-loop thread; reading under that lock could deadlock against a garbage-collection finalizer that acquires the same lock on that thread. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) +- Fixed a deadlock when exporting a Zarr file to HDF5 with `HDF5IO.export`. A zarr array is now read into memory before the HDF5 write (in `__list_fill__`/`__scalar_fill__`), so the read does not run while h5py's global lock is held. @rly [#1547](https://github.com/hdmf-dev/hdmf/pull/1547) - Fixed the Jupyter HTML representation (`_repr_html_`) rendering a scalar numpy `bool` or `int` attribute (e.g. `np.bool_`, `np.int64`, as read back from an HDF5 attribute) as an expandable "array" block reporting `Shape: ()`, while `float` and `str` scalars rendered inline. `_unwrap_scalar` now also unwraps numpy scalars (`np.generic`) so every scalar renders inline consistently. @h-mayorquin [#1546](https://github.com/hdmf-dev/hdmf/pull/1546) - Fixed subclassing a `MultiContainerInterface` type without redefining `__init__`. A subclass now inherits the ancestor's constructor (custom or auto-generated) instead of regenerating one that drops the parent's docval arguments. This restores the common "generate with `get_class`, subclass to customize one method, re-register with `@register_class`" extension idiom for `DynamicTable`-family subtypes. @rly [#1540](https://github.com/hdmf-dev/hdmf/pull/1540) - Fixed `ObjectMapper._parse_isoformat` raising `ValueError: Invalid isoformat string` when reading an ISO 8601 timestamp that ends in the `Z` (UTC) designator on Python < 3.11. `datetime.fromisoformat` did not accept a trailing `Z` until Python 3.11, but `requires-python` is `>=3.10` and hdmf's own writer emits the equivalent `+00:00` offset, so a `Z`-terminated timestamp written by a peer tool was unreadable on the supported 3.10 floor. A trailing `Z`/`z` is now normalized to `+00:00` before parsing. @Leonard013 [#1539](https://github.com/hdmf-dev/hdmf/pull/1539) From 2083ad1b6dd0618996874011428fccffedc865a5 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:46:29 -0700 Subject: [PATCH 7/9] Remove the zarr v3 materialize test that broke collection without zarr The test's skipUnless decorator referenced zarr.__version__ at class-body evaluation time, raising NameError during collection in environments where zarr is not installed. The test was also correctness-neutral (the materialize change prevents a concurrency deadlock, not a wrong result) and always skipped under the zarr v2 pin used in CI, so it provided no regression coverage. The materialize code path stays covered by the existing zarr-input round-trip tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_io_hdf5_h5tools.py | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index 56f088565..22bb2cc33 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -3763,23 +3763,6 @@ def test_roundtrip_basic(self): self.assertListEqual(foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist(), read_foofile.buckets['bucket1'].foos['foo1'].my_data[:].tolist()) - @unittest.skipUnless(int(zarr.__version__.split(".")[0]) >= 3, "requires zarr v3") - def test_write_zarr_v3_dataset_materialized(self): - # Regression: a zarr array is read into memory before the HDF5 write, so the read - # does not run while the HDF5 lock is held (see HDF5IO.__list_fill__). The deadlock - # this avoids is specific to zarr v3, which reads on a background thread. - base_data = np.arange(50).astype('int32') - store = os.path.join(self.zarr_path, 'arr.zarr') - z = zarr.create_array(store=store, shape=base_data.shape, chunks=(8,), dtype='int32') - z[:] = base_data - with HDF5IO(self.path, mode='a') as io: - f = io._file - io.write_dataset(f, DatasetBuilder(name='test_dataset', data=z, attributes={})) - dset = f['test_dataset'] - self.assertTupleEqual(tuple(dset.shape), base_data.shape) - self.assertEqual(dset.dtype, base_data.dtype) - self.assertListEqual(dset[:].tolist(), base_data.tolist()) - def test_roundtrip_empty_dataset(self): zarr.save(self.zarr_path, np.asarray([]).astype('int64')) zarr_data = zarr.open(self.zarr_path, 'r') From 933b8af727a7fd99a1b9d1a13ad01f23dd639edd Mon Sep 17 00:00:00 2001 From: Ryan Ly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:38:22 -0700 Subject: [PATCH 8/9] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/hdmf/backends/io.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/hdmf/backends/io.py b/src/hdmf/backends/io.py index 13c1bb58a..21b936fcc 100644 --- a/src/hdmf/backends/io.py +++ b/src/hdmf/backends/io.py @@ -270,10 +270,16 @@ def __del__(self): still_open = bool(is_open()) if callable(is_open) else False except Exception: still_open = False - if still_open: + if not still_open: + return + + try: + source = getattr(self, "source", None) warn( f"{type(self).__name__} was not closed before being garbage collected. " "Use a context manager or call close() to release resources. " - f"Source: {self.source!r}", + f"Source: {source!r}", ResourceWarning, ) + except Exception: + pass From 98d5f06ec33aed27a93a06fee25b01a9950f98e4 Mon Sep 17 00:00:00 2001 From: rly <310197+rly@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:40:14 -0700 Subject: [PATCH 9/9] Isolate the open-IO registry in the atexit cleanup test Swap a fresh WeakSet into hdmf.backends.io._open_ios for the duration of the test so the simulated interpreter-exit cleanup only closes the IO created here, not IOs left open by other tests (addresses a PR review comment). Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/unit/test_io_hdf5_h5tools.py | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_io_hdf5_h5tools.py b/tests/unit/test_io_hdf5_h5tools.py index 22bb2cc33..8fbc58bef 100644 --- a/tests/unit/test_io_hdf5_h5tools.py +++ b/tests/unit/test_io_hdf5_h5tools.py @@ -3,6 +3,7 @@ import os import unittest import warnings +import weakref from io import BytesIO from pathlib import Path import shutil @@ -4556,10 +4557,17 @@ def test_del_does_not_warn_after_close(self): self.assertEqual(len(resource_warnings), 0) def test_open_io_registered_for_atexit_cleanup(self): - from hdmf.backends.io import _open_ios, _close_open_ios + import hdmf.backends.io as io_module - io = HDF5IO(self.path, manager=self.manager, mode='w') - self.assertIn(io, _open_ios) - self.assertTrue(io.is_open()) - _close_open_ios() # simulate interpreter-exit cleanup - self.assertFalse(io.is_open()) + # Isolate the registry so the simulated cleanup only touches the IO created here, + # not IOs left open by other tests (which would couple test order and mask leaks). + original_open_ios = io_module._open_ios + io_module._open_ios = weakref.WeakSet() + try: + io = HDF5IO(self.path, manager=self.manager, mode='w') + self.assertIn(io, io_module._open_ios) + self.assertTrue(io.is_open()) + io_module._close_open_ios() # simulate interpreter-exit cleanup + self.assertFalse(io.is_open()) + finally: + io_module._open_ios = original_open_ios