fix: read ProxyCommand stdin with a blocking os.read instead of select() - #158
Open
0Cymantek0 wants to merge 1 commit into
Open
0Cymantek0 wants to merge 1 commit into
0Cymantek0 wants to merge 1 commit into
Conversation
Windows select() only accepts sockets, so in --proxy-mode the stdin pump died with OSError before any input was forwarded, leaving the runtime without a stdin. A blocking os.read() is equivalent on POSIX (the select waited on just this one fd with no timeout) and works everywhere. The pump direction had no coverage (the existing bridge test stubs the reader thread out), so this adds test_bridge_proxy_mode_pumps_stdin_to_ws, which fails with the select() call in place on Windows and passes with the blocking read on every platform.
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
1 participant
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.
On Windows the stdin pump in
colab ssh --proxy-modedies at startup.select()only accepts sockets there, so the call on the stdin fd raises OSError, the pump thread exits, and the runtime never receives any input. ssh hangs with no way to type.The select() call waited on a single fd with no timeout, so a blocking
os.read()is equivalent on POSIX and also works on Windows. The pump direction had no test coverage (the existing bridge test stubs the reader thread out), so this addstest_bridge_proxy_mode_pumps_stdin_to_ws. It fails with the select() call in place on Windows and passes with the blocking read.Part of the Windows breakage in #85 and #100. The termios crash at startup is separate and already covered by #135.
Verified on Windows 11 with Python 3.12: ruff check clean, all ssh bridge tests pass, and the full suite shows no new failures (the remaining ones are pre-existing platform issues: SIGHUP, prompt_toolkit win32, rich help sorting).
Merge ready.