Skip to content

Fix listening socket leak in AgentLocalProxy.get_connection - #2694

Open
kratos0718 wants to merge 1 commit into
paramiko:mainfrom
kratos0718:fix-agent-local-proxy-socket-leak
Open

kratos0718 wants to merge 1 commit into
paramiko:mainfrom
kratos0718:fix-agent-local-proxy-socket-leak

Conversation

@kratos0718

@kratos0718 kratos0718 commented Sep 24, 2026 •

Copy link
Copy Markdown

What

AgentLocalProxy.get_connection() opens a unix socket, binds and listens on it, accepts exactly one connection, and returns that accepted socket:

def get_connection(self):
    conn = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
    try:
        conn.bind(self._agent._get_filename())
        conn.listen(1)
        (r, addr) = conn.accept()
        return r, addr
    except:
        raise

The listening socket (conn) itself is never closed on either the success path or the exception path - the except: raise doesn't help, it's a no-op for control flow (catches and immediately re-raises the same exception, identical behavior to no try/except at all). Every call leaks the listening socket's file descriptor.

Fix

    finally:
        conn.close()

Closes the listening socket unconditionally. The already-accepted connection socket (r) is untouched - accept() returns an independent fd, closing the listener doesn't affect it.

Testing

Added a regression test in tests/agent.py that drives this through a real unix socket end to end: a background thread connects while the main thread calls get_connection(), and the test tags whichever socket instance get_connection() calls bind() on (via a small socket.socket subclass swapped in for the duration of the call - unambiguous regardless of how the connector thread's own retrying client sockets interleave with it, since only the listening socket ever calls bind()), then asserts that socket is closed (fileno() == -1) while the accepted one isn't.

Verified this test fails against the pre-fix code (fileno() returns a live fd, not -1) and passes against the fix. Full tests/agent.py suite: 33 passed, 1 skipped, unchanged from before this PR. flake8/black --check clean on both changed files.

Note

#2644 is an open PR touching the same file's other bare-except: clauses (a separate, broader cleanup, unrelated to this specific leak) - flagging in case of overlap, though this PR only touches the one get_connection method.

get_connection() opens a unix socket, binds and listens on it, accepts
exactly one connection, and returns that accepted socket - but the
listening socket itself was never closed on either the success or the
exception path (the bare except: raise was a no-op for control flow,
so it didn't help either). Every call leaked the listening socket's
file descriptor.

Replaced the bare except:/raise with a finally: conn.close(), which
closes the listening socket unconditionally while leaving the
already-accepted connection socket untouched (accept() returns an
independent fd, closing the listener doesn't affect it).

Added a regression test using a real unix socket end to end: a
background thread connects while the main thread calls
get_connection(), and the test asserts the listening socket (tagged
via a bind()-intercepting socket subclass, identified unambiguously
regardless of how the connector thread's own retrying client sockets
interleave with it) is closed afterward, while the accepted socket is
not. Verified this test fails against the pre-fix code (fileno() is
a live fd, not -1) and passes against the fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant