fix(connection): bound session/channel reconnect retry loops#182
Open
plusky wants to merge 1 commit into
Open
Conversation
run() and __sftp_reconnect() retried session/channel creation in an unbounded while-loop calling reconnect(), which is a no-op while the transport reports active. A transport that stays active but keeps refusing new sessions/channels (server MaxSessions reached, half-broken transport) made both loops spin with no sleep or attempt cap, pinning a CPU forever. Cap both loops at _RECONNECT_MAX_ATTEMPTS with a short backoff (mirroring wait_reconnect) and force a full transport teardown/reconnect once attempts reach _RECONNECT_FORCE_AFTER even while is_active() is True, so a degraded-but-active transport is recycled. A reconnect that fails on the teardown path is logged and swallowed (wait_reconnect's idiom) so it consumes one attempt of the same budget instead of escaping as a raw connect error. Raise SSHException when the cap is exhausted instead of looping; run()'s docstring documents the new raise. Tests pin the recycle path (teardown at the force threshold followed by a successful reconnect lets run() complete), the budget consumption when reconnect keeps failing, and the cap-exhausted SSHException on both the session and SFTP loops.
This was referenced Jul 5, 2026
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.
What
run()and__sftp_reconnect()retried session/channel creation inunbounded
whileloops whose only recovery action,reconnect(), is ano-op while the transport still reports active. A transport that stays
active but keeps refusing new sessions/channels (server
MaxSessionsreached, half-broken transport) made both loops spin forever with no
sleep and no attempt cap, pinning a CPU.
Both loops are now bounded:
_RECONNECT_MAX_ATTEMPTS(10) attempts witha 1s backoff, mirroring
wait_reconnect; once attempts reach_RECONNECT_FORCE_AFTER(3) the transport is forcibly torn down andreconnected even while
is_active()is true, so a degraded-but-activetransport gets recycled. A reconnect that fails on the teardown path is
logged and consumes one attempt of the same budget (wait_reconnect's
idiom) instead of escaping as a raw connect error. When the budget is
exhausted,
paramiko.SSHExceptionis raised;run()'s docstringdocuments the new raise.
Note: this is one of four independent single-concern fixes touching
nearby regions of
repose/connection.pyin this batch (boundedreconnect loops, channel close on abort, prompt serialization,
accept-new persistence). They can merge in any order; later ones may
need a trivial rebase, which I'll provide.
Verification
Full gate green (ruff format/check, ty, pytest: 551 passed, coverage
83.92%). The recycle mechanism is mutation-verified: deleting the
force-teardown block, or unwrapping the reconnect try/except, fails the
new tests. Tests pin (a) recovery success — teardown at the force
threshold followed by a working reconnect lets
run()complete andasserts
connect()was called, (b) budget consumption when reconnectkeeps failing ends in the cap-exhausted
SSHException, not a rawconnect error, and (c) cap exhaustion on both the session and SFTP
loops. Reviewed by a fan-out adversarial code review; all confirmed
findings addressed.
AI-assisted.