Skip to content

test(core): run the fd-leak check on success, not only after a failure - #2583

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:core-tests-checks-never-run
Open

test(core): run the fd-leak check on success, not only after a failure#2583
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:core-tests-checks-never-run

Conversation

@LeSingh1

Copy link
Copy Markdown
Contributor

Problem

CheckFDLeaks.__exit__ compares the descriptor count under an inverted guard (test_leaks.py:128-133):

def__exit__(self, exc_type, exc_val, exc_tb):
ifexc_typeisnotNone:
gc.collect()
final_fds=self.process.num_fds()
assertfinal_fds==self.initial_fdsreturnFalse

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:

deftest_alloc_handle(ipc_memory_resource):
"""Check for fd leaks in allocation_handle."""mr=ipc_memory_resourcewithCheckFDLeaks():
[mr.allocation_handlefor_inrange(10)]

test_alloc_handle and the 12 parametrizations of test_pass_object (4 object kinds × 3 launchers, covering the success, launch-failure and reduce-failure paths) contain no assertion of their own. All 13 therefore pass unconditionally today: self.initial_fds is recorded in __enter__ and then discarded. A descriptor leak in allocation_handle, in Buffer / mr / ipc_descriptor pickling, or on either failure path produces no failure.

On the one branch where it did run, the assert was also actively harmful: raising from __exit__replaces the exception the test was really reporting, so a genuine error surfaces as a bare fd-count mismatch.

Fix

Guard on exc_type is None, and put the counts in the assertion message so a failure names the leak size instead of just assert 41 == 38.

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. I have no GPU here and cannot tell you either way — see below.

What I ran

Environment: macOS, no CUDA driver and no CUDA toolkit. These tests additionally require Linux (USING_FDS = platform.system() == "Linux"), psutil, and a working IPC memory resource, so they are triple-unrunnable here.

  • Did not run:cuda_core/tests/memory_ipc/test_leaks.py.
  • Ran: a reduction of the context manager with a fake fd counter, keeping the guard verbatim:
 before clean body -> passed silently
before body leaks 3 fds -> passed silently <-- the defect
after clean body -> passed silently
after body leaks 3 fds -> detected the leak
  • Ran:python -m py_compile, ruff check, ruff format --check on the changed file — clean, no new findings against a main baseline.
  • Checked:CheckFDLeaks has exactly two users, test_alloc_handle (:31) and test_pass_object (:102), and neither has any other assertion; prime() in __enter__ exists to warm up the allocations so the baseline is stable, which only makes sense for a check that runs on success.

`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.
@copy-pr-bot

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actionsgithub-actionsBot added the cuda.core Everything related to the cuda.core module label Aug 9, 2026
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cuda.coreEverything related to the cuda.core module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@LeSingh1