Skip to content

Commit b8dfdb6

Browse files
nicolas-grekasnicolas-grekas
committed
Merge branch '7.4' into 8.0
* 7.4: fix merge [HttpFoundation] Fix tests [JsonStreamer] Finish #62063 upmerge [Console] Fix signal handlers not being cleared after command termination [HttpFoundation] Fix RequestTest insulation ReflectionMethod::setAccessible() is no-op since PHP 8.1 CS fix fix merge
2 parents 490ebd9 + a2b78ae commit b8dfdb6

File tree

4 files changed

+313
-18
lines changed

4 files changed

+313
-18
lines changed

‎Application.php‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -997,9 +997,13 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
997997
}
998998
}
999999

1000+
$registeredSignals = false;
10001001
if (($commandSignals = $command->getSubscribedSignals()) || $this->dispatcher && $this->signalsToDispatchEvent) {
10011002
$signalRegistry = $this->getSignalRegistry();
10021003

1004+
$registeredSignals = true;
1005+
$this->getSignalRegistry()->pushCurrentHandlers();
1006+
10031007
if ($this->dispatcher) {
10041008
// We register application signals, so that we can dispatch the event
10051009
foreach ($this->signalsToDispatchEvent as $signal) {
@@ -1056,7 +1060,13 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
10561060
}
10571061

10581062
if (null === $this->dispatcher) {
1059-
return $command->run($input, $output);
1063+
try {
1064+
return $command->run($input, $output);
1065+
} finally {
1066+
if ($registeredSignals) {
1067+
$this->getSignalRegistry()->popPreviousHandlers();
1068+
}
1069+
}
10601070
}
10611071

10621072
// bind before the console.command event, so the listeners have access to input options/arguments
@@ -1086,6 +1096,10 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
10861096
if (0 === $exitCode = $event->getExitCode()) {
10871097
$e = null;
10881098
}
1099+
} finally {
1100+
if ($registeredSignals) {
1101+
$this->getSignalRegistry()->popPreviousHandlers();
1102+
}
10891103
}
10901104

10911105
$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: 179 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2313,6 +2313,26 @@ private function runRestoresSttyTest(array $params, int $expectedExitCode, bool
23132313
}
23142314
}
23152315

2316+
#[RequiresPhpExtension('pcntl')]
2317+
public function testSignalHandlersAreCleanedUpAfterCommandRuns()
2318+
{
2319+
$application = new Application();
2320+
$application->setAutoExit(false);
2321+
$application->setCatchExceptions(false);
2322+
$application->addCommand(new SignableCommand(false));
2323+
2324+
$signalRegistry = $application->getSignalRegistry();
2325+
$tester = new ApplicationTester($application);
2326+
2327+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should be empty initially.');
2328+
2329+
$tester->run(['command' => 'signal']);
2330+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should be empty after first run.');
2331+
2332+
$tester->run(['command' => 'signal']);
2333+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Registry should still be empty after second run.');
2334+
}
2335+
23162336
#[RequiresPhpExtension('pcntl')]
23172337
public function testSignalableInvokableCommand()
23182338
{
@@ -2329,6 +2349,40 @@ public function testSignalableInvokableCommand()
23292349
$this->assertTrue($invokable->signaled);
23302350
}
23312351

2352+
#[RequiresPhpExtension('pcntl')]
2353+
public function testSignalHandlersCleanupOnException()
2354+
{
2355+
$command = new class('signal:exception') extends Command implements SignalableCommandInterface {
2356+
public function getSubscribedSignals(): array
2357+
{
2358+
return [\SIGUSR1];
2359+
}
2360+
2361+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2362+
{
2363+
return false;
2364+
}
2365+
2366+
protected function execute(InputInterface $input, OutputInterface $output): int
2367+
{
2368+
throw new \RuntimeException('Test exception');
2369+
}
2370+
};
2371+
2372+
$application = new Application();
2373+
$application->setAutoExit(false);
2374+
$application->setCatchExceptions(true);
2375+
$application->addCommand($command);
2376+
2377+
$signalRegistry = $application->getSignalRegistry();
2378+
$tester = new ApplicationTester($application);
2379+
2380+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Pre-condition: Registry must be empty.');
2381+
2382+
$tester->run(['command' => 'signal:exception']);
2383+
$this->assertCount(0, $this->getHandlersForSignal($signalRegistry, \SIGUSR1), 'Signal handlers must be cleaned up even on exception.');
2384+
}
2385+
23322386
#[RequiresPhpExtension('pcntl')]
23332387
public function testSignalableInvokableCommandThatExtendsBaseCommand()
23342388
{
@@ -2428,6 +2482,90 @@ public function testAlarmableCommandWithoutInterval()
24282482
$this->assertFalse($command->signaled);
24292483
}
24302484

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

2586+
#[RequiresPhpExtension('pcntl')]
2587+
public function testOriginalHandlerRestoredAfterPop()
2588+
{
2589+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'Pre-condition: Original handler for SIGUSR1 must be SIG_DFL.');
2590+
2591+
$application = new Application();
2592+
$application->setAutoExit(false);
2593+
$application->setCatchExceptions(false);
2594+
$application->addCommand(new SignableCommand(false));
2595+
2596+
$tester = new ApplicationTester($application);
2597+
$tester->run(['command' => 'signal']);
2598+
2599+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'OS-level handler for SIGUSR1 must be restored to SIG_DFL.');
2600+
2601+
$tester->run(['command' => 'signal']);
2602+
$this->assertSame(\SIG_DFL, pcntl_signal_get_handler(\SIGUSR1), 'OS-level handler must remain SIG_DFL after a second run.');
2603+
}
2604+
24482605
#[RequiresPhpExtension('pcntl')]
24492606
#[TestWith([false])]
24502607
#[TestWith([4])]
@@ -2491,18 +2648,6 @@ public function onAlarm(ConsoleAlarmEvent $event): void
24912648
$this->assertSame([SignalEventSubscriber::class, AlarmEventSubscriber::class], $command->signalHandlers);
24922649
}
24932650

2494-
private function createSignalableApplication(Command $command, ?EventDispatcherInterface $dispatcher): Application
2495-
{
2496-
$application = new Application();
2497-
$application->setAutoExit(false);
2498-
if ($dispatcher) {
2499-
$application->setDispatcher($dispatcher);
2500-
}
2501-
$application->addCommand(new LazyCommand($command->getName(), [], '', false, fn () => $command, true));
2502-
2503-
return $application;
2504-
}
2505-
25062651
public function testShellVerbosityIsRestoredAfterCommandExecutionWithInitialValue()
25072652
{
25082653
// Set initial SHELL_VERBOSITY
@@ -2598,6 +2743,28 @@ public function testShellVerbosityDoesNotLeakBetweenCommandExecutions()
25982743
$this->assertArrayNotHasKey('SHELL_VERBOSITY', $_ENV);
25992744
$this->assertArrayNotHasKey('SHELL_VERBOSITY', $_SERVER);
26002745
}
2746+
2747+
/**
2748+
* Reads the private "signalHandlers" property of the SignalRegistry for assertions.
2749+
*/
2750+
public function getHandlersForSignal(SignalRegistry $registry, int $signal): array
2751+
{
2752+
$handlers = (\Closure::bind(fn () => $this->signalHandlers, $registry, SignalRegistry::class))();
2753+
2754+
return $handlers[$signal] ?? [];
2755+
}
2756+
2757+
private function createSignalableApplication(Command $command, ?EventDispatcherInterface $dispatcher): Application
2758+
{
2759+
$application = new Application();
2760+
$application->setAutoExit(false);
2761+
if ($dispatcher) {
2762+
$application->setDispatcher($dispatcher);
2763+
}
2764+
$application->addCommand(new LazyCommand($command->getName(), [], '', false, fn () => $command, true));
2765+
2766+
return $application;
2767+
}
26012768
}
26022769

26032770
class CustomApplication extends Application

0 commit comments

Comments
 (0)