Skip to content

Fix intermittent failure of test/leaky-handles.py - #4932

Merged
bdbaddog merged 2 commits into
SCons:masterfrom
bdbaddog:fix-leaky-handles
Oct 18, 2026
Merged

bdbaddog merged 2 commits into
SCons:masterfrom
bdbaddog:fix-leaky-handles

Conversation

@bdbaddog

Copy link
Copy Markdown
Contributor

The test counted the entries in /proc//fd using a shell pipeline. $$ is the shell, not ls, and for a pipeline the shell creates a pipe, forks both commands, and only then closes its own copies of the pipe ends. When ls reached getdents() first it saw those too and printed 4 or 5 instead of 3, failing the exact stdout match. Measured at roughly 0.5% when the shell and ls contend for a cpu (2 failures in 400 runs of the test pinned to one cpu), which is why it showed up under parallel and loaded test runs but never in isolation.

The forked command now reports the inherited handles itself, so no shell pipe is involved: 0 failures in 400 runs under the same conditions.

This also restores what the test is meant to check. The handle it leaked could not reach the child for two unrelated reasons: the 'with' block closed it when the SConstruct finished being read, well before the build phase forked the command, and handles are close-on-exec by default since Python 3.4 (PEP 446) regardless of what SCons does. Binding the file object to an SConscript global is not enough either, as it is garbage collected before the build phase; the test now keeps an os-level handle open and marks it inheritable. Verified it fails as intended when close_fds is removed from SCons/Platform/posix.py.

Contributor Checklist:

  • I have created a new test or updated the unit tests to cover the new/changed functionality.
  • I have updated CHANGES.txt and RELEASE.txt (and read the README.rst).
  • I have updated the appropriate documentation

The test counted the entries in /proc/<shell pid>/fd using a shell
pipeline. $$ is the shell, not ls, and for a pipeline the shell creates
a pipe, forks both commands, and only then closes its own copies of the
pipe ends. When ls reached getdents() first it saw those too and printed
4 or 5 instead of 3, failing the exact stdout match. Measured at roughly
0.5% when the shell and ls contend for a cpu (2 failures in 400 runs of
the test pinned to one cpu), which is why it showed up under parallel
and loaded test runs but never in isolation.

The forked command now reports the inherited handles itself, so no shell
pipe is involved: 0 failures in 400 runs under the same conditions.

This also restores what the test is meant to check. The handle it leaked
could not reach the child for two unrelated reasons: the 'with' block
closed it when the SConstruct finished being read, well before the build
phase forked the command, and handles are close-on-exec by default since
Python 3.4 (PEP 446) regardless of what SCons does. Binding the file
object to an SConscript global is not enough either, as it is garbage
collected before the build phase; the test now keeps an os-level handle
open and marks it inheritable. Verified it fails as intended when
close_fds is removed from SCons/Platform/posix.py.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bdbaddog
bdbaddog requested a review from mwichmann October 18, 2026 01:58
@bdbaddog
bdbaddog merged commit 280b154 into SCons:master Oct 18, 2026
10 of 12 checks passed
@bdbaddog
bdbaddog deleted the fix-leaky-handles branch October 18, 2026 22:29
@mwichmann mwichmann added the testsuite Things that only affect the SCons testing. Do not use just because a PR has tests. label Oct 18, 2026
@mwichmann mwichmann added this to the NextRelease milestone Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testsuite Things that only affect the SCons testing. Do not use just because a PR has tests.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants