Skip to content

Commit ddb064e

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '6.4' into 7.3
* 6.4: [Console] Fix signal handlers not being cleared after command termination [HttpFoundation] Fix RequestTest insulation ReflectionMethod::setAccessible() is no-op since PHP 8.1
2 parents c28ad91 + 32d9216 commit ddb064e

File tree

4 files changed

+321
-18
lines changed

4 files changed

+321
-18
lines changed

‎Application.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -993,9 +993,13 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
993993
}
994994
}
995995

996+
$registeredSignals = false;
996997
if (($commandSignals = $command->getSubscribedSignals()) || $this->dispatcher && $this->signalsToDispatchEvent) {
997998
$signalRegistry = $this->getSignalRegistry();
998999

1000+
$registeredSignals = true;
1001+
$this->getSignalRegistry()->pushCurrentHandlers();
1002+
9991003
if ($this->dispatcher) {
10001004
// We register application signals, so that we can dispatch the event
10011005
foreach ($this->signalsToDispatchEvent as $signal) {
@@ -1052,7 +1056,13 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
10521056
}
10531057

10541058
if (null === $this->dispatcher) {
1055-
return $command->run($input, $output);
1059+
try {
1060+
return $command->run($input, $output);
1061+
} finally {
1062+
if ($registeredSignals) {
1063+
$this->getSignalRegistry()->popPreviousHandlers();
1064+
}
1065+
}
10561066
}
10571067

10581068
// bind before the console.command event, so the listeners have access to input options/arguments
@@ -1082,6 +1092,10 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
10821092
if (0 === $exitCode = $event->getExitCode()) {
10831093
$e = null;
10841094
}
1095+
} finally {
1096+
if ($registeredSignals) {
1097+
$this->getSignalRegistry()->popPreviousHandlers();
1098+
}
10851099
}
10861100

10871101
$event = new ConsoleTerminateEvent($command, $input, $output, $exitCode);

‎SignalRegistry/SignalRegistry.php‎

Lines changed: 56 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,21 @@
1313

1414
final class SignalRegistry
1515
{
16+
/**
17+
* @var array<int, array<callable>>
18+
*/
1619
private array $signalHandlers = [];
1720

21+
/**
22+
* @var array<array<int, array<callable>>>
23+
*/
24+
private array $stack = [];
25+
26+
/**
27+
* @var array<int, callable|int|string>
28+
*/
29+
private array $originalHandlers = [];
30+
1831
public function __construct()
1932
{
2033
if (\function_exists('pcntl_async_signals')) {
@@ -24,17 +37,21 @@ public function __construct()
2437

2538
public function register(int $signal, callable $signalHandler): void
2639
{
27-
if (!isset($this->signalHandlers[$signal])) {
28-
$previousCallback = pcntl_signal_get_handler($signal);
40+
$previous = pcntl_signal_get_handler($signal);
41+
42+
if (!isset($this->originalHandlers[$signal])) {
43+
$this->originalHandlers[$signal] = $previous;
44+
}
2945

30-
if (\is_callable($previousCallback)) {
31-
$this->signalHandlers[$signal][] = $previousCallback;
46+
if (!isset($this->signalHandlers[$signal])) {
47+
if (\is_callable($previous) && [$this, 'handle'] !== $previous) {
48+
$this->signalHandlers[$signal][] = $previous;
3249
}
3350
}
3451

3552
$this->signalHandlers[$signal][] = $signalHandler;
3653

37-
pcntl_signal($signal, $this->handle(...));
54+
pcntl_signal($signal, [$this, 'handle']);
3855
}
3956

4057
public static function isSupported(): bool
@@ -55,6 +72,40 @@ public function handle(int $signal): void
5572
}
5673
}
5774

75+
/**
76+
* Pushes the current active handlers onto the stack and clears the active list.
77+
*
78+
* This prepares the registry for a new set of handlers within a specific scope.
79+
*
80+
* @internal
81+
*/
82+
public function pushCurrentHandlers(): void
83+
{
84+
$this->stack[] = $this->signalHandlers;
85+
$this->signalHandlers = [];
86+
}
87+
88+
/**
89+
* Restores the previous handlers from the stack, making them active.
90+
*
91+
* This also restores the original OS-level signal handler if no
92+
* more handlers are registered for a signal that was just popped.
93+
*
94+
* @internal
95+
*/
96+
public function popPreviousHandlers(): void
97+
{
98+
$popped = $this->signalHandlers;
99+
$this->signalHandlers = array_pop($this->stack) ?? [];
100+
101+
// Restore OS handler if no more Symfony handlers for this signal
102+
foreach ($popped as $signal => $handlers) {
103+
if (!($this->signalHandlers[$signal] ?? false) && isset($this->originalHandlers[$signal])) {
104+
pcntl_signal($signal, $this->originalHandlers[$signal]);
105+
}
106+
}
107+
}
108+
58109
/**
59110
* @internal
60111
*/

‎Tests/ApplicationTest.php‎

Lines changed: 187 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2288,6 +2288,28 @@ private function runRestoresSttyTest(array $params, int $expectedExitCode, bool
22882288
}
22892289
}
22902290

2291+
/**
2292+
* @requires extension pcntl
2293+
*/
2294+
public function testSignalHandlersAreCleanedUpAfterCommandRuns()
2295+
{
2296+
$application = new Application();
2297+
$application->setAutoExit(false);
2298+
$application->setCatchExceptions(false);
2299+
$application->add(new SignableCommand(false));
2300+
2301+
$signalRegistry = $application->getSignalRegistry();
2302+
$tester = new ApplicationTester($application);
2303+
2304+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should be empty initially.');
2305+
2306+
$tester->run(['command' => 'signal']);
2307+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should be empty after first run.');
2308+
2309+
$tester->run(['command' => 'signal']);
2310+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should still be empty after second run.');
2311+
}
2312+
22912313
/**
22922314
* @requires extension pcntl
22932315
*/
@@ -2306,6 +2328,42 @@ public function testSignalableInvokableCommand()
23062328
$this->assertTrue($invokable->signaled);
23072329
}
23082330

2331+
/**
2332+
* @requires extension pcntl
2333+
*/
2334+
public function testSignalHandlersCleanupOnException()
2335+
{
2336+
$command = new class('signal:exception') extends Command implements SignalableCommandInterface {
2337+
public function getSubscribedSignals(): array
2338+
{
2339+
return [\SIGUSR1];
2340+
}
2341+
2342+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2343+
{
2344+
return false;
2345+
}
2346+
2347+
protected function execute(InputInterface $input, OutputInterface $output): int
2348+
{
2349+
throw new \RuntimeException('Test exception');
2350+
}
2351+
};
2352+
2353+
$application = new Application();
2354+
$application->setAutoExit(false);
2355+
$application->setCatchExceptions(true);
2356+
$application->add($command);
2357+
2358+
$signalRegistry = $application->getSignalRegistry();
2359+
$tester = new ApplicationTester($application);
2360+
2361+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Pre-condition: Registry must be empty.');
2362+
2363+
$tester->run(['command' => 'signal:exception']);
2364+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Signal handlers must be cleaned up even on exception.');
2365+
}
2366+
23092367
/**
23102368
* @requires extension pcntl
23112369
*/
@@ -2417,6 +2475,92 @@ public function testAlarmableCommandWithoutInterval()
24172475
$this->assertFalse($command->signaled);
24182476
}
24192477

2478+
/**
2479+
* @requires extension pcntl
2480+
*/
2481+
public function testNestedCommandsIsolateSignalHandlers()
2482+
{
2483+
$application = new Application();
2484+
$application->setAutoExit(false);
2485+
$application->setCatchExceptions(false);
2486+
2487+
$signalRegistry = $application->getSignalRegistry();
2488+
$self = $this;
2489+
2490+
$innerCommand = new class('signal:inner') extends Command implements SignalableCommandInterface {
2491+
public $signalRegistry;
2492+
public $self;
2493+
2494+
public function getSubscribedSignals(): array
2495+
{
2496+
return [\SIGUSR1];
2497+
}
2498+
2499+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2500+
{
2501+
return false;
2502+
}
2503+
2504+
protected function execute(InputInterface $input, OutputInterface $output): int
2505+
{
2506+
$handlers = $this->self->getHandlersForSignal($this->signalRegistry, \SIGUSR1);
2507+
$this->self->assertCount(1, $handlers, 'Inner command should only see its own handler.');
2508+
$output->write('Inner execute.');
2509+
2510+
return 0;
2511+
}
2512+
};
2513+
2514+
$outerCommand = new class('signal:outer') extends Command implements SignalableCommandInterface {
2515+
public $signalRegistry;
2516+
public $self;
2517+
2518+
public function getSubscribedSignals(): array
2519+
{
2520+
return [\SIGUSR1];
2521+
}
2522+
2523+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2524+
{
2525+
return false;
2526+
}
2527+
2528+
protected function execute(InputInterface $input, OutputInterface $output): int
2529+
{
2530+
$handlersBefore = $this->self->getHandlersForSignal($this->signalRegistry, \SIGUSR1);
2531+
$this->self->assertCount(1, $handlersBefore, 'Outer command must have its handler registered.');
2532+
2533+
$output->write('Outer pre-run.');
2534+
2535+
$this->getApplication()->find('signal:inner')->run(new ArrayInput([]), $output);
2536+
2537+
$output->write('Outer post-run.');
2538+
2539+
$handlersAfter = $this->self->getHandlersForSignal($this->signalRegistry, \SIGUSR1);
2540+
$this->self->assertCount(1, $handlersAfter, 'Outer command\'s handler must be restored.');
2541+
$this->self->assertSame($handlersBefore, $handlersAfter, 'Handler stack must be identical after pop.');
2542+
2543+
return 0;
2544+
}
2545+
};
2546+
2547+
$innerCommand->self = $self;
2548+
$innerCommand->signalRegistry = $signalRegistry;
2549+
$outerCommand->self = $self;
2550+
$outerCommand->signalRegistry = $signalRegistry;
2551+
2552+
$application->add($innerCommand);
2553+
$application->add($outerCommand);
2554+
2555+
$tester = new ApplicationTester($application);
2556+
2557+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Pre-condition: Registry must be empty.');
2558+
$tester->run(['command' => 'signal:outer']);
2559+
$this->assertStringContainsString('Outer pre-run.Inner execute.Outer post-run.', $tester->getDisplay());
2560+
2561+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry must be empty after all commands are finished.');
2562+
}
2563+
24202564
/**
24212565
* @requires extension pcntl
24222566
*/
@@ -2436,6 +2580,27 @@ public function testAlarmableCommandHandlerCalledAfterEventListener()
24362580
$this->assertSame([AlarmEventSubscriber::class, AlarmableCommand::class], $command->signalHandlers);
24372581
}
24382582

2583+
/**
2584+
* @requires extension pcntl
2585+
*/
2586+
public function testOriginalHandlerRestoredAfterPop()
2587+
{
2588+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'Pre-condition: Original handler for SIGUSR1 must be SIG_DFL.');
2589+
2590+
$application = new Application();
2591+
$application->setAutoExit(false);
2592+
$application->setCatchExceptions(false);
2593+
$application->add(new SignableCommand(false));
2594+
2595+
$tester = new ApplicationTester($application);
2596+
$tester->run(['command' => 'signal']);
2597+
2598+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'OS-level handler for SIGUSR1 must be restored to SIG_DFL.');
2599+
2600+
$tester->run(['command' => 'signal']);
2601+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'OS-level handler must remain SIG_DFL after a second run.');
2602+
}
2603+
24392604
/**
24402605
* @requires extension pcntl
24412606
*
@@ -2485,18 +2650,6 @@ public function onAlarm(ConsoleAlarmEvent $event): void
24852650
$this->assertSame([SignalEventSubscriber::class, AlarmEventSubscriber::class], $command->signalHandlers);
24862651
}
24872652

2488-
private function createSignalableApplication(Command $command, ?EventDispatcherInterface $dispatcher): Application
2489-
{
2490-
$application = new Application();
2491-
$application->setAutoExit(false);
2492-
if ($dispatcher) {
2493-
$application->setDispatcher($dispatcher);
2494-
}
2495-
$application->add(new LazyCommand($command->getName(), [], '', false, fn () => $command, true));
2496-
2497-
return $application;
2498-
}
2499-
25002653
public function testShellVerbosityIsRestoredAfterCommandExecutionWithInitialValue()
25012654
{
25022655
// Set initial SHELL_VERBOSITY
@@ -2592,6 +2745,28 @@ public function testShellVerbosityDoesNotLeakBetweenCommandExecutions()
25922745
$this->assertArrayNotHasKey('SHELL_VERBOSITY', $_ENV);
25932746
$this->assertArrayNotHasKey('SHELL_VERBOSITY', $_SERVER);
25942747
}
2748+
2749+
/**
2750+
* Reads the private "signalHandlers" property of the SignalRegistry for assertions.
2751+
*/
2752+
public function getHandlersForSignal(SignalRegistry $registry, int $signal): array
2753+
{
2754+
$handlers = (\Closure::bind(fn () => $this->signalHandlers, $registry, SignalRegistry::class))();
2755+
2756+
return $handlers[$signal] ?? [];
2757+
}
2758+
2759+
private function createSignalableApplication(Command $command, ?EventDispatcherInterface $dispatcher): Application
2760+
{
2761+
$application = new Application();
2762+
$application->setAutoExit(false);
2763+
if ($dispatcher) {
2764+
$application->setDispatcher($dispatcher);
2765+
}
2766+
$application->add(new LazyCommand($command->getName(), [], '', false, fn () => $command, true));
2767+
2768+
return $application;
2769+
}
25952770
}
25962771

25972772
class CustomApplication extends Application

0 commit comments

Comments
 (0)