Skip to content

Commit 74c95be

Browse files
authored
fix(shell): terminate the process on timeout (#574)
1 parent 36d8587 commit 74c95be

File tree

3 files changed

+47
-2
lines changed

3 files changed

+47
-2
lines changed

docs/component/shell.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
#### `Functions`
1414

15-
- [execute](./../../src/Psl/Shell/execute.php#L44)
15+
- [execute](./../../src/Psl/Shell/execute.php#L45)
1616
- [stream_unpack](./../../src/Psl/Shell/stream_unpack.php#L30)
1717
- [unpack](./../../src/Psl/Shell/unpack.php#L16)
1818

src/Psl/Shell/execute.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use function pack;
2020
use function proc_close;
2121
use function proc_open;
22+
use function proc_terminate;
2223
use function strpbrk;
2324

2425
/**
@@ -146,6 +147,7 @@ static function (array $m) use (&$environment, &$variable_cache, &$variable_coun
146147
$stderr = new IO\CloseReadStreamHandle($pipes[2]);
147148

148149
$code = 0;
150+
$timed_out = false;
149151
try {
150152
$result = '';
151153

@@ -157,16 +159,23 @@ static function (array $m) use (&$environment, &$variable_cache, &$variable_coun
157159
$result .= pack('C1N1', $type, Str\Byte\length($chunk)) . $chunk;
158160
}
159161
} catch (IO\Exception\TimeoutException $previous) {
162+
$timed_out = true;
163+
160164
throw new Exception\TimeoutException(
161165
'reached timeout while the process output is still not readable.',
162166
0,
163167
$previous,
164168
);
165169
} finally {
166170
$stdout->close();
167-
168171
$stderr->close();
169172

173+
// Kill the process before closing if it timed out, otherwise
174+
// proc_close() will block indefinitely waiting for it to exit.
175+
if ($timed_out) {
176+
proc_terminate($process, 9);
177+
}
178+
170179
$code = proc_close($process);
171180
}
172181

tests/unit/Shell/ExecuteTest.php

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
namespace Psl\Tests\Unit\Shell;
66

77
use PHPUnit\Framework\TestCase;
8+
use Psl\DateTime;
9+
use Psl\DateTime\Duration;
810
use Psl\Env;
911
use Psl\OS;
1012
use Psl\SecureRandom;
@@ -141,4 +143,38 @@ public function testErrorOutputIsPacked(): void
141143
static::assertSame('hello', $stdout);
142144
static::assertSame(' world', $stderr);
143145
}
146+
147+
public function testTimeoutDoesNotAffectFastCommands(): void
148+
{
149+
$result = Shell\execute(PHP_BINARY, ['-r', 'echo "hello";'], timeout: Duration::seconds(5));
150+
151+
static::assertSame('hello', $result);
152+
}
153+
154+
public function testTimeoutException(): void
155+
{
156+
if (OS\is_windows()) {
157+
static::markTestSkipped(
158+
'Timeout relies on IO\\streaming() which behaves differently on Windows with non-blocking pipes.',
159+
);
160+
}
161+
162+
$start = DateTime\Timestamp::monotonic();
163+
164+
try {
165+
Shell\execute(PHP_BINARY, ['-r', 'sleep(10);'], timeout: Duration::seconds(2));
166+
} catch (Shell\Exception\TimeoutException $_) {
167+
$elapsed = DateTime\Timestamp::monotonic()->since($start);
168+
169+
static::assertLessThan(
170+
3.0,
171+
$elapsed->getTotalSeconds(),
172+
'Process was not killed after timeout — proc_close() likely blocked.',
173+
);
174+
175+
return;
176+
}
177+
178+
static::fail('Expected timeout exception');
179+
}
144180
}

0 commit comments

Comments
 (0)