Repository navigation
Fix SSH tunnels not forwarding when the task holds over 1024 file descriptors - #74299
Open
firasbouzazi wants to merge 2 commits into
Open
firasbouzazi wants to merge 2 commits into
firasbouzazi wants to merge 2 commits into
Conversation
…criptors SSHTunnel's forwarding loop waited with select.select(), which cannot watch a descriptor numbered 1024 or higher. The resulting ValueError ended the forwarding thread, so SSHHook.get_tunnel() left a local port that accepted connections but never forwarded them.
Contributor
Author
The test polled with a sleep loop whose event was misleadingly named a deadline; a helper now waits for the condition until a real monotonic deadline.
Contributor
Author
|
The failing jobs look unrelated to this change: |
This branch has not been deployed
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.
Follow-up to #74211.
SSHTunnel._serve_foreverwaited withselect.select(), which cannot watch a descriptor numbered 1024 or higher. TheValueErrorwas caught and ended the forwarding thread, so with that many descriptors openSSHHook.get_tunnel()left a local port that accepted connections but never forwarded them.The loop now uses
selectors.DefaultSelector. Since connections come and go, the listening and shutdown sockets are registered once, and each forwarded connection's socket and channel are registered when accepted and unregistered before they are closed. Unregistering uses the descriptor recorded at registration: a closed socket has no descriptor, andparamiko.Channel.fileno()on a closed channel would create a new pipe instead of returning the old one. Connections whose channel the remote end closed are now closed and unregistered rather than only dropped from the list.Tests: the in-process paramiko server fixtures from #74211 move to a shared
conftest.pyand now echo forwarded (direct-tcpip) channels. New tests forward data throughSSHTunnelsequentially and concurrently while the process holds over 1024 descriptors (no data gets through before the fix), and check that connections closed by either end are unregistered. Against a local sshd,SSHHook.get_tunnel()with 1,100 extra descriptors open went from 0/10 to 10/10 round trips. The SSH provider unit tests (209) pass.related: #74205
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code following the guidelines