Skip to content

Fix race conditions and stale SSH client process in SshSocksConnector - #54

Merged
clue merged 4 commits into
clue:1.xfrom
clue-labs:socks-race
Oct 17, 2026
Merged

clue merged 4 commits into
clue:1.xfrom
clue-labs:socks-race

Conversation

@clue

@clue clue commented Oct 17, 2026

Copy link
Copy Markdown
Owner

This changeset fixes several race conditions and a stale SSH client process in SshSocksConnector. These date back to the shared SSH client process from #8 and only show when a connection attempt is cancelled or the SSH client exits at the wrong moment:

  • Cancelling while the SSH client fails would crash with Call to a member function then() on null.
  • An SSH client exiting late would clear the reference to a newer one, which then kept running.
  • Cancelling before the SSH client was ready would still open a SOCKS connection and leak it.
  • An SSH client dying after it was ready went unnoticed, so connection attempts kept failing until all connections were closed.

Each fix comes in its own commit with a test reproducing the original problem.

Builds on top of #52, #50 and #8
Refs clue/reactphp-redis#105 and friends-of-reactphp/mysql#110

@clue clue added the bug Something isn't working label Oct 17, 2026
@clue clue added this to the v1.5.0 milestone Oct 17, 2026
@clue
clue requested a balanced review from Copilot October 17, 2026 21:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The stale-process regression test must distinguish connector-triggered termination from natural process exit.

1 open issue
What changed in this PR

Fixes shared SSH process lifecycle races in SshSocksConnector.

Changes:

  • Prevents cancelled attempts from opening SOCKS connections.
  • Protects replacement process references and clears exited processes.
  • Adds regression tests for cancellation and process-exit timing.
File Description
tests/​IntegrationSshSocksConnectorTest.php Adds four lifecycle regression tests.
src/​SshSocksConnector.php Corrects cancellation, idle cleanup, and process-exit handling.

Review effort: Balanced

Comment thread tests/IntegrationSshSocksConnectorTest.php
@clue
clue merged commit 08e32e2 into clue:1.x Oct 17, 2026
16 checks passed
@clue
clue deleted the socks-race branch October 17, 2026 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants