Skip to content

Commit 6f8dabc

Browse files
Don't double-close the child pty fd (fixes parallel Bad file descriptor) (#3975) (#3976)
## Summary Fixes #3975 — a regression in 4.56.2 (from #3974) where parallel runs intermittently fail with `Bad file descriptor` / `Input/output error`. ## Root cause In `get_stream_file_no` (the pty path used when running under a tty), the child fd is closed once the child has inherited it, and then closed **again** on generator teardown: ```python main_fd, child_fd = allocated_pty try: yield child_fd os.close(child_fd) # (1) close once inherited yield main_fd finally: for fd in (child_fd, main_fd): # (2) closes child_fd a *second* time with suppress(OSError): os.close(fd) ``` In a serial run the second `os.close(child_fd)` is a harmless suppressed `EBADF`. But under parallel execution, between (1) and (2) the freed fd **number** can be reused by a sibling run — so (2) closes *that* run's descriptor instead, causing the intermittent `Bad file descriptor` / `Input/output error`. ## Fix Track whether the child fd was already closed and skip it in the teardown loop, so each fd is closed exactly once. The master fd is still released (no leak), and the early-teardown path (before the child fd is inherited) still closes both. ## Tests - Added `test_get_stream_file_no_closes_each_pty_fd_once`: it fails on `main` (the child fd is closed twice: `[22, 22, 11]`) and passes here. - The existing pty fd tests (`test_local_subprocess_tty_closes_master_fd`, `test_pty_closes_fds_when_termios_fails`) still pass; full `tests/execute/local_subprocess/test_local_subprocess.py` is green (50 passed, 1 skipped). `ruff check` clean. Changelog fragment added. *Disclosure: prepared with AI assistance; reviewed and verified locally.* --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 5458a28 commit 6f8dabc

3 files changed

Lines changed: 31 additions & 2 deletions

File tree

docs/changelog/3975.bugfix.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Stop double-closing the child pty file descriptor when running under a tty, which could race a parallel run that had
2+
reused the freed descriptor number and cause intermittent ``Bad file descriptor``/``Input/output error`` failures - by
3+
:user:`apoorvdarshan`.

src/tox/execute/local_sub_process/__init__.py

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,13 +267,17 @@ def get_stream_file_no(key: str) -> Generator[int, Popen[bytes], None]:
267267
allocated_pty = _pty(key)
268268
if allocated_pty is not None:
269269
main_fd, child_fd = allocated_pty
270+
child_fd_open = True
270271
try:
271272
yield child_fd
272273
os.close(child_fd) # close the child process pipe once the child inherited it
274+
child_fd_open = False
273275
yield main_fd
274276
finally:
275-
# close on generator teardown; the master fd is not a process stream so nobody else closes it
276-
for fd in (child_fd, main_fd):
277+
# close on generator teardown; the master fd is not a process stream so nobody else closes it.
278+
# Skip the child fd if it was already closed above: re-closing a freed fd number can race with a
279+
# parallel run that has since reused it, corrupting the sibling's fd (see #3975).
280+
for fd in (child_fd, main_fd) if child_fd_open else (main_fd,):
277281
with suppress(OSError):
278282
os.close(fd)
279283
else:

tests/execute/local_subprocess/test_local_subprocess.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,28 @@ def test_local_subprocess_tty_closes_master_fd(monkeypatch: MonkeyPatch, mocker:
387387
os.fstat(fd)
388388

389389

390+
@pytest.mark.skipif(sys.platform == "win32", reason="pty is Unix-only")
391+
def test_get_stream_file_no_closes_each_pty_fd_once(mocker: MockerFixture) -> None:
392+
"""Each pty fd must be closed exactly once.
393+
394+
The child fd is closed once the child has inherited it; closing it a second time on generator teardown can race a
395+
parallel run that has reused the freed fd number, corrupting the sibling's fd (see #3975).
396+
397+
"""
398+
main_fd, child_fd = 11, 22
399+
mocker.patch.object(local_sub_process, "_pty", return_value=(main_fd, child_fd))
400+
close_spy = mocker.patch.object(os, "close")
401+
402+
gen = LocalSubProcessExecuteInstance.get_stream_file_no("stdout")
403+
assert next(gen) == child_fd
404+
assert gen.send(MagicMock()) == main_fd
405+
gen.close()
406+
407+
closed = [call.args[0] for call in close_spy.call_args_list]
408+
assert closed.count(child_fd) == 1
409+
assert closed.count(main_fd) == 1
410+
411+
390412
@pytest.mark.skipif(sys.platform != "win32", reason="overlapped I/O reader is Windows-only")
391413
def test_read_via_thread_windows_stops_while_read_pending(mocker: MockerFixture) -> None:
392414
"""A never-completing overlapped read must not keep the reader thread alive after stop is set."""

0 commit comments

Comments
 (0)