From 847fcd9e75f9d706e84901c38ec3e67277a7d906 Mon Sep 17 00:00:00 2001 From: LeSingh1 Date: Sun, 9 Aug 2026 14:09:10 -0700 Subject: [PATCH] test(core): run the fd-leak check on success, not only after a failure `CheckFDLeaks.__exit__` compares the descriptor count under an inverted guard: def __exit__(self, exc_type, exc_val, exc_tb): if exc_type is not None: gc.collect() final_fds = self.process.num_fds() assert final_fds == self.initial_fds return False `exc_type is not None` means "the with-body raised". So the comparison runs only when the test has already failed for some other reason, and never on the normal path -- which is the only path its two users take. Both of them delegate their entire assertion power to this class: def test_alloc_handle(ipc_memory_resource): mr = ipc_memory_resource with CheckFDLeaks(): [mr.allocation_handle for _ in range(10)] `test_alloc_handle` and the 12 parametrizations of `test_pass_object` (4 object kinds x 3 launchers, covering the success, launch-failure and reduce-failure paths) contain no assertion of their own, so all 13 currently pass unconditionally. An fd leak in `allocation_handle`, in Buffer / mr / ipc_descriptor pickling, or on either failure path produces no failure -- `self.initial_fds` is recorded in `__enter__` and then discarded. On the branch it did run, the assert was also harmful: raising from `__exit__` replaces the exception the test was really reporting, so a genuine error would surface as a bare fd-count mismatch. Guard on `exc_type is None` and add the counts to the assertion message. Note for reviewers: this makes a check live that has never executed. If it now fails, that is the leak it was written to catch, not a defect in this change. --- cuda_core/tests/memory_ipc/test_leaks.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cuda_core/tests/memory_ipc/test_leaks.py b/cuda_core/tests/memory_ipc/test_leaks.py index c6e44824137..8638648b643 100644 --- a/cuda_core/tests/memory_ipc/test_leaks.py +++ b/cuda_core/tests/memory_ipc/test_leaks.py @@ -126,10 +126,17 @@ def __enter__(self): return self def __exit__(self, exc_type, exc_val, exc_tb): - if exc_type is not None: + # Check on a CLEAN exit only. `exc_type is not None` inverted this: the + # count was compared only when the body had already raised -- never on + # the path the two users of this class actually take -- and there the + # AssertionError replaced the exception the test was really reporting. + if exc_type is None: gc.collect() final_fds = self.process.num_fds() - assert final_fds == self.initial_fds + assert final_fds == self.initial_fds, ( + f"leaked {final_fds - self.initial_fds} file descriptor(s): " + f"{self.initial_fds} open before the block, {final_fds} after" + ) return False