Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
clue merged 4 commits into from
Oct 17, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11d6772
Fix fatal error when SSH client exits before idle timer fires
clue Oct 17, 2026
f602b08
Fix orphaned SSH client process when previous process exits late
clue Oct 17, 2026
d485d1e
Fix leaking SOCKS connection when cancelling before SSH client is ready
clue Oct 17, 2026
bab5249
Fix reusing stale SSH client process after it exits unexpectedly
clue Oct 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions src/SshSocksConnector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,8 @@ public function connect($uri)
}

// no listening SSH client process started? start one and remember reference
// if spawning SSH client process fails, clear reference for next start
if ($this->listen === null) {
$listen =& $this->listen;
$this->listen = $this->listen()->then(null, function ($e) use (&$listen) {
$listen = null;
throw $e;
});
$this->listen = $this->listen();

// @codeCoverageIgnoreStart
if ($this->debug) {
Expand Down Expand Up @@ -146,8 +141,12 @@ public function connect($uri)
throw new \RuntimeException('Connection to ' . $uri . ' cancelled while waiting for SSH client');
});

$this->listen->then(function () use ($uri, $that, $socks, $deferred, &$connecting) {
// SSH client process started => start SOCKS connection
$this->listen->then(function () use ($uri, $that, $socks, $deferred, &$connecting, &$cancelled) {
// SSH client process started => start SOCKS connection (unless connection attempt has already been cancelled)
if ($cancelled) {
return;
}

$connecting = $socks->connect($uri)->then(function (ConnectionInterface $connection) use ($that, $deferred) {
// connection to SOCKS server successful => consider idle when connection closes again
$connection->on('close', function () use ($that) {
Expand Down Expand Up @@ -194,14 +193,15 @@ public function idle()

if ($this->pending < 1 && $this->idleTimer === null && $this->listen !== null) {
$listen =& $this->listen;
$promise = $this->listen;
$debug = $this->debug;
$this->idleTimer = $this->loop->addTimer($this->idlePeriod, function () use (&$listen, $debug) {
$listen->then(function (Process $process) {
$this->idleTimer = $this->loop->addTimer($this->idlePeriod, function () use (&$listen, $promise, $debug) {
$promise->then(function (Process $process) {
$process->terminate();
}, function () {
// ignore to avoid reporting unhandled rejection
});
$listen->cancel();
$promise->cancel();
$listen = null;

// @codeCoverageIgnoreStart
Expand Down Expand Up @@ -255,13 +255,20 @@ private function listen()
$process->terminate();
});

$process->on('exit', function ($code) use ($deferred, &$last) {
// if SSH client process exits, clear reference for next start unless already replaced
$listen =& $this->listen;
$promise = $deferred->promise();
$process->on('exit', function ($code) use ($deferred, &$last, &$listen, $promise) {
if ($listen === $promise) {
$listen = null;
}

$deferred->reject(new \RuntimeException(
$last,
(int) $code
));
});

return $deferred->promise();
return $promise;
}
}
153 changes: 153 additions & 0 deletions tests/IntegrationSshSocksConnectorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,159 @@ public function testConnectCancellationWillRejectWithException()
Loop::run();
}

public function testConnectCancellationWillRejectWithExceptionWhenProcessExitsBeforeIdleTimeout()
{
$connector = new SshSocksConnector('host');

$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'exec false; #');

// delay idle timer, so process exit is reported before idle timer fires
$ref = new \ReflectionProperty($connector, 'idlePeriod');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 0.2);

$promise = $connector->connect('example.com:80');
$promise->cancel();

$promise->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

Loop::run();
}

public function testConnectCancellationWillNotInvokeSocksConnectorWhenProcessBecomesReadyForOtherPendingConnection()
{
$connector = new SshSocksConnector('host');

$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'echo "debug1: channel 0: new [port listener]" >&2; exec sleep 10; #');

$socks = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$socks->expects($this->once())->method('connect')->with('example.com:81')->willReturn(\React\Promise\reject(new \RuntimeException('Connection refused')));

$ref = new \ReflectionProperty($connector, 'socks');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, $socks);

$first = $connector->connect('example.com:80');
$first->cancel();

$first->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

$second = $connector->connect('example.com:81');
$second->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

Loop::run();
}

public function testConnectWillStartNewProcessWhenPreviousProcessExitsAfterPortListenerMessage()
{
$connector = new SshSocksConnector('host');

$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'echo "debug1: channel 0: new [port listener]" >&2; exec sleep 0.1; #');

$deferred = new Deferred();
$socks = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$socks->expects($this->once())->method('connect')->with('example.com:80')->willReturn($deferred->promise());

$ref = new \ReflectionProperty($connector, 'socks');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, $socks);

$connector->connect('example.com:80');

$onRejected = $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'));
Loop::addTimer(0.5, function () use ($connector, $onRejected) {
$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'echo "Connection closed by remote host" >&2; exit 255; #');

$promise = $connector->connect('example.com:80');
$promise->then(null, $onRejected);
});

Loop::run();
}

public function testConnectWillTerminateNewProcessWhenPreviousProcessExitsLateAfterIdleTimeout()
{
$connector = new SshSocksConnector('host');

$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'trap "sleep 0.2; exit 0" TERM; while :; do sleep 0.01; done; #');

$deferred = new Deferred();
$socks = $this->getMockBuilder('React\Socket\ConnectorInterface')->getMock();
$socks->expects($this->once())->method('connect')->with('example.com:81')->willReturn($deferred->promise());

$ref = new \ReflectionProperty($connector, 'socks');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, $socks);

// first process only exits 0.2s after being terminated by the idle timer
$promise = $connector->connect('example.com:80');
$promise->cancel();

$promise->then(null, $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException')));

$process = null;
$onRejected = $this->expectCallableOnceWith($this->isInstanceOf('RuntimeException'));
Loop::addTimer(0.05, function () use ($connector, &$process, $onRejected) {
$ref = new \ReflectionProperty($connector, 'cmd');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$ref->setValue($connector, 'echo "debug1: channel 0: new [port listener]" >&2; exec sleep 2; #');

$promise = $connector->connect('example.com:81');
$promise->then(null, $onRejected);

$ref = new \ReflectionProperty($connector, 'listen');
if (PHP_VERSION_ID < 80100) {
$ref->setAccessible(true);
}
$listen = $ref->getValue($connector);
assert($listen instanceof PromiseInterface);

$listen->then(function ($value) use (&$process) {
$process = $value;
});
});

// second process becomes idle only after first process exited
Loop::addTimer(0.5, function () use ($deferred) {
$deferred->reject(new \RuntimeException('Connection refused'));
});

Loop::run();

$this->assertInstanceOf('React\ChildProcess\Process', $process);
$this->assertTrue($process->isTerminated());
Comment thread
clue marked this conversation as resolved.
}

public function testConnectWillRejectWithExceptionAndTerminateProcessWhenProcessOutputsLineExceedingMaximumBufferSize()
{
$connector = new SshSocksConnector('host');
Expand Down