Use a private duplicate of stdout/stderr fd in CliRunner#3387
Closed
charlieleith wants to merge 1 commit intopallets:mainfrom
Closed
Use a private duplicate of stdout/stderr fd in CliRunner#3387charlieleith wants to merge 1 commit intopallets:mainfrom
charlieleith wants to merge 1 commit intopallets:mainfrom
Conversation
Since 8.3.3, ``_NamedTextIOWrapper.fileno()`` returns the file descriptor of the stream being replaced. Code under test that calls ``os.dup2`` over that value (logging tees, build tools, subprocess wrappers, ...) ends up redirecting the host process's real stdout/stderr fd, breaking outer harnesses such as pytest's fd-based capture. Reproduce with the snippet from issue pallets#3384: pytest exits with ``OSError: Illegal seek`` from ``_pytest.capture`` while tearing down its own capture. Mirror pytest's own ``capfd`` model: ``os.dup`` the original fd inside ``CliRunner`` and expose the duplicate via ``fileno()``. ``os.dup2`` against the duplicate now only redirects this private copy, leaving the host's stdout/stderr intact. Close the duplicates when isolation exits.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes #3384
Summary
Click 8.3.3 (#3244) started exposing the original stream's file descriptor via
_NamedTextIOWrapper.fileno(), so that C-level consumers likefaulthandlerandsubprocesscould obtain a real fd fromsys.stdout/sys.stderrinsideCliRunnerisolation.That regressed any command that performs
os.dup2against the value returned bysys.stdout.fileno()— logging tees, build tools, subprocess wrappers, etc. The redirect happens on the host process's real stdout fd, which clobbers any outer fd-based capture (most notably pytest'scapfd/capsyscapture pipe). The reporter's repro fails during pytest's own teardown withOSError: [Errno 29] Illegal seekbecause pytest tries toseek(0)on the tmpfile that has just been overwritten with a non-seekable pipe.This mirrors how pytest itself handles the same problem in
capfd:os.dupthe original fd insideCliRunnerand expose the duplicate viafileno().os.dup2against the duplicate redirects only this private copy, so the host's stdout/stderr — and any capture installed on top of it — stays intact. The duplicates are closed when the isolation context exits.The semantics from #3244 are preserved:
faulthandler.enable(),subprocess, signal handlers, and similar C-level consumers still get a valid, terminal-bound fd.Repro
The repro from the issue passes after this change:
Tests
test_dup2_over_stdout_fd_does_not_leakintests/test_testing.py. Fails onmainwithOSError: Illegal seekduring pytest teardown (same backtrace as the issue), passes with this change.test_faulthandler_enablestill passes — duped fd remains valid forfaulthandler.1437 passed, 23 skipped, 1 xfailed.Checklist
_NamedTextIOWrapperdocstring +versionchanged).CHANGES.rstsummarizing the change and linking to the issue.versionchangedentries in any relevant code docs.