Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ad86f72

Browse filesBrowse files
nicolas-grekasnicolas-grekas
committed
feature #49529 [Console] Add support for managing exit code while handling signals (lyrixx)
This PR was merged into the 6.3 branch. Discussion ---------- [Console] Add support for managing exit code while handling signals | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | yes | New feature? | yes | Deprecations? | yes (new method in interrface) | Tickets | Fix #48340 | License | MIT | Doc PR | Signal handling is hard! This commit fixes an issue introduced in bb7779d, see this [comment](#48299 (comment)) for more information. Symfony registers by default 4 signals, the most common ones: `SIGINT`, `SIGTERM`, `SIGUSR1`, and `SIGUSR2`. When no signal handler are registered, the default behavior is to stop the execution of the current script, to replication PHP default behavior. That's why we had a call to exit(0), before bb7779d and that's why it must be restored. However, the concerns raised in the [issue](#47809) are valid, and we should provided a solution for this. Now we have the following features: * By default, we exit, like PHP does by default on `SIGINT`, `SIGTERM`; * It's possible to tell via the event to not exit, by calling `$event->abortExit()`; * It's possible to tell via the event to exit with a custom code, by calling `$event->setExitCode($code)`; * It's possible, via the `SignalableCommandInterface` to tell via the command to not exit, by returning `int|false` from `handleSigbal()` method * Fixed case when there is not event to dispatch at all, but the command register some I think we have all we need now. I added more tests to ensure we don't break anything in the future. --- Sorry for the massive ping here, but this is bit like an a house of cards to make it work in any case. I hope I didn't miss anything. ping `@akuzia` `@lcobucci` `@GromNaN` `@olegpro` Commits ------- 1650e38 [Console] Add support for managing exit code while handling signals
2 parents 5b1a20d + 1650e38 commit ad86f72
Copy full SHA for ad86f72

11 files changed

+354
-103
lines changed

‎.github/expected-missing-return-types.diff

Copy file name to clipboardExpand all lines: .github/expected-missing-return-types.diff
+84-74Lines changed: 84 additions & 74 deletions
Large diffs are not rendered by default.

‎UPGRADE-6.3.md

Copy file name to clipboardExpand all lines: UPGRADE-6.3.md
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
UPGRADE FROM 6.2 to 6.3
22
=======================
33

4+
Console
5+
-------
6+
7+
* Return int or false from `SignalableCommandInterface::handleSignal()` instead
8+
of void and add a second argument `$previousExitCode`
9+
410
DependencyInjection
511
-------------------
612

‎src/Symfony/Component/Console/Application.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Application.php
+42-17Lines changed: 42 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,37 +1000,62 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
10001000
}
10011001
}
10021002

1003-
if ($this->signalsToDispatchEvent) {
1004-
$commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : [];
1005-
1006-
if ($commandSignals || null !== $this->dispatcher) {
1007-
if (!$this->signalRegistry) {
1008-
throw new RuntimeException('Unable to subscribe to signal events. Make sure that the `pcntl` extension is installed and that "pcntl_*" functions are not disabled by your php.ini\'s "disable_functions" directive.');
1009-
}
1003+
$commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : [];
1004+
if ($commandSignals || $this->dispatcher && $this->signalsToDispatchEvent) {
1005+
if (!$this->signalRegistry) {
1006+
throw new RuntimeException('Unable to subscribe to signal events. Make sure that the `pcntl` extension is installed and that "pcntl_*" functions are not disabled by your php.ini\'s "disable_functions" directive.');
1007+
}
10101008

1011-
if (Terminal::hasSttyAvailable()) {
1012-
$sttyMode = shell_exec('stty -g');
1009+
if (Terminal::hasSttyAvailable()) {
1010+
$sttyMode = shell_exec('stty -g');
10131011

1014-
foreach ([\SIGINT, \SIGTERM] as $signal) {
1015-
$this->signalRegistry->register($signal, static function () use ($sttyMode) {
1016-
shell_exec('stty '.$sttyMode);
1017-
});
1018-
}
1012+
foreach ([\SIGINT, \SIGTERM] as $signal) {
1013+
$this->signalRegistry->register($signal, static fn () => shell_exec('stty '.$sttyMode));
10191014
}
10201015
}
10211016

1022-
if (null !== $this->dispatcher) {
1017+
if ($this->dispatcher) {
1018+
// We register application signals, so that we can dispatch the event
10231019
foreach ($this->signalsToDispatchEvent as $signal) {
10241020
$event = new ConsoleSignalEvent($command, $input, $output, $signal);
10251021

1026-
$this->signalRegistry->register($signal, function () use ($event) {
1022+
$this->signalRegistry->register($signal, function ($signal) use ($event, $command, $commandSignals) {
10271023
$this->dispatcher->dispatch($event, ConsoleEvents::SIGNAL);
1024+
$exitCode = $event->getExitCode();
1025+
1026+
// If the command is signalable, we call the handleSignal() method
1027+
if (\in_array($signal, $commandSignals, true)) {
1028+
$exitCode = $command->handleSignal($signal, $exitCode);
1029+
// BC layer for Symfony <= 5
1030+
if (null === $exitCode) {
1031+
trigger_deprecation('symfony/console', '6.3', 'Not returning an exit code from "%s::handleSignal()" is deprecated, return "false" to keep the command running or "0" to exit successfully.', get_debug_type($command));
1032+
$exitCode = 0;
1033+
}
1034+
}
1035+
1036+
if (false !== $exitCode) {
1037+
exit($exitCode);
1038+
}
10281039
});
10291040
}
1041+
1042+
// then we register command signals, but not if already handled after the dispatcher
1043+
$commandSignals = array_diff($commandSignals, $this->signalsToDispatchEvent);
10301044
}
10311045

10321046
foreach ($commandSignals as $signal) {
1033-
$this->signalRegistry->register($signal, [$command, 'handleSignal']);
1047+
$this->signalRegistry->register($signal, function (int $signal) use ($command): void {
1048+
$exitCode = $command->handleSignal($signal);
1049+
// BC layer for Symfony <= 5
1050+
if (null === $exitCode) {
1051+
trigger_deprecation('symfony/console', '6.3', 'Not returning an exit code from "%s::handleSignal()" is deprecated, return "false" to keep the command running or "0" to exit successfully.', get_debug_type($command));
1052+
$exitCode = 0;
1053+
}
1054+
1055+
if (false !== $exitCode) {
1056+
exit($exitCode);
1057+
}
1058+
});
10341059
}
10351060
}
10361061

‎src/Symfony/Component/Console/CHANGELOG.md

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/CHANGELOG.md
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ CHANGELOG
44
6.3
55
---
66

7-
* Remove `exit` call in `Application` signal handlers. Commands will no longer be automatically interrupted after receiving signal other than `SIGUSR1` or `SIGUSR2`
7+
* Add support for choosing exit code while handling signal, or to not exit at all
88
* Add `ProgressBar::setPlaceholderFormatter` to set a placeholder attached to a instance, instead of being global.
99
* Add `ReStructuredTextDescriptor`
1010

‎src/Symfony/Component/Console/Command/SignalableCommandInterface.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Command/SignalableCommandInterface.php
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ public function getSubscribedSignals(): array;
2525

2626
/**
2727
* The method will be called when the application is signaled.
28+
*
29+
* @param int|false $previousExitCode
30+
31+
* @return int|false The exit code to return or false to continue the normal execution
2832
*/
29-
public function handleSignal(int $signal): void;
33+
public function handleSignal(int $signal, /* int|false $previousExitCode = 0 */);
3034
}

‎src/Symfony/Component/Console/Event/ConsoleSignalEvent.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Event/ConsoleSignalEvent.php
+22-1Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,36 @@
2121
final class ConsoleSignalEvent extends ConsoleEvent
2222
{
2323
private int $handlingSignal;
24+
private int|false $exitCode;
2425

25-
public function __construct(Command $command, InputInterface $input, OutputInterface $output, int $handlingSignal)
26+
public function __construct(Command $command, InputInterface $input, OutputInterface $output, int $handlingSignal, int|false $exitCode = 0)
2627
{
2728
parent::__construct($command, $input, $output);
2829
$this->handlingSignal = $handlingSignal;
30+
$this->exitCode = $exitCode;
2931
}
3032

3133
public function getHandlingSignal(): int
3234
{
3335
return $this->handlingSignal;
3436
}
37+
38+
public function setExitCode(int $exitCode): void
39+
{
40+
if ($exitCode < 0 || $exitCode > 255) {
41+
throw new \InvalidArgumentException('Exit code must be between 0 and 255.');
42+
}
43+
44+
$this->exitCode = $exitCode;
45+
}
46+
47+
public function abortExit(): void
48+
{
49+
$this->exitCode = false;
50+
}
51+
52+
public function getExitCode(): int|false
53+
{
54+
return $this->exitCode;
55+
}
3556
}

‎src/Symfony/Component/Console/Tests/ApplicationTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Tests/ApplicationTest.php
+115-3Lines changed: 115 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use Symfony\Component\Console\Command\SignalableCommandInterface;
2121
use Symfony\Component\Console\CommandLoader\CommandLoaderInterface;
2222
use Symfony\Component\Console\CommandLoader\FactoryCommandLoader;
23+
use Symfony\Component\Console\ConsoleEvents;
2324
use Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass;
2425
use Symfony\Component\Console\Event\ConsoleCommandEvent;
2526
use Symfony\Component\Console\Event\ConsoleErrorEvent;
@@ -1929,7 +1930,8 @@ public function testSignalListener()
19291930

19301931
$dispatcherCalled = false;
19311932
$dispatcher = new EventDispatcher();
1932-
$dispatcher->addListener('console.signal', function () use (&$dispatcherCalled) {
1933+
$dispatcher->addListener('console.signal', function (ConsoleSignalEvent $e) use (&$dispatcherCalled) {
1934+
$e->abortExit();
19331935
$dispatcherCalled = true;
19341936
});
19351937

@@ -1978,6 +1980,34 @@ public function testSignalSubscriber()
19781980
$this->assertTrue($subscriber2->signaled);
19791981
}
19801982

1983+
/**
1984+
* @requires extension pcntl
1985+
*/
1986+
public function testSignalDispatchWithoutEventToDispatch()
1987+
{
1988+
$command = new SignableCommand();
1989+
1990+
$application = $this->createSignalableApplication($command, null);
1991+
$application->setSignalsToDispatchEvent();
1992+
1993+
$this->assertSame(1, $application->run(new ArrayInput(['signal'])));
1994+
$this->assertTrue($command->signaled);
1995+
}
1996+
1997+
/**
1998+
* @requires extension pcntl
1999+
*/
2000+
public function testSignalDispatchWithoutEventDispatcher()
2001+
{
2002+
$command = new SignableCommand();
2003+
2004+
$application = $this->createSignalableApplication($command, null);
2005+
$application->setSignalsToDispatchEvent(\SIGUSR1);
2006+
2007+
$this->assertSame(1, $application->run(new ArrayInput(['signal'])));
2008+
$this->assertTrue($command->signaled);
2009+
}
2010+
19812011
/**
19822012
* @requires extension pcntl
19832013
*/
@@ -2077,9 +2107,36 @@ public function testSignalableCommandDoesNotInterruptedOnTermSignals()
20772107
$application->setAutoExit(false);
20782108
$application->setDispatcher($dispatcher);
20792109
$application->add($command);
2110+
20802111
$this->assertSame(129, $application->run(new ArrayInput(['signal'])));
20812112
}
20822113

2114+
public function testSignalableWithEventCommandDoesNotInterruptedOnTermSignals()
2115+
{
2116+
if (!\defined('SIGINT')) {
2117+
$this->markTestSkipped('SIGINT not available');
2118+
}
2119+
2120+
$command = new TerminatableWithEventCommand();
2121+
2122+
$dispatcher = new EventDispatcher();
2123+
$dispatcher->addSubscriber($command);
2124+
$application = new Application();
2125+
$application->setAutoExit(false);
2126+
$application->setDispatcher($dispatcher);
2127+
$application->add($command);
2128+
$tester = new ApplicationTester($application);
2129+
$this->assertSame(51, $tester->run(['signal']));
2130+
$expected = <<<EOTXT
2131+
Still processing...
2132+
["handling event",2,0]
2133+
["exit code",2,125]
2134+
Wrapping up, wait a sec...
2135+
2136+
EOTXT;
2137+
$this->assertSame($expected, $tester->getDisplay(true));
2138+
}
2139+
20832140
/**
20842141
* @group tty
20852142
*/
@@ -2217,10 +2274,12 @@ public function getSubscribedSignals(): array
22172274
return SignalRegistry::isSupported() ? [\SIGUSR1] : [];
22182275
}
22192276

2220-
public function handleSignal(int $signal): void
2277+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
22212278
{
22222279
$this->signaled = true;
22232280
$this->signalHandlers[] = __CLASS__;
2281+
2282+
return false;
22242283
}
22252284
}
22262285

@@ -2232,10 +2291,61 @@ public function getSubscribedSignals(): array
22322291
return SignalRegistry::isSupported() ? [\SIGINT] : [];
22332292
}
22342293

2235-
public function handleSignal(int $signal): void
2294+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
22362295
{
22372296
$this->signaled = true;
22382297
$this->signalHandlers[] = __CLASS__;
2298+
2299+
return false;
2300+
}
2301+
}
2302+
2303+
#[AsCommand(name: 'signal')]
2304+
class TerminatableWithEventCommand extends Command implements SignalableCommandInterface, EventSubscriberInterface
2305+
{
2306+
private bool $shouldContinue = true;
2307+
private OutputInterface $output;
2308+
2309+
protected function execute(InputInterface $input, OutputInterface $output): int
2310+
{
2311+
$this->output = $output;
2312+
2313+
for ($i = 0; $i <= 10 && $this->shouldContinue; ++$i) {
2314+
$output->writeln('Still processing...');
2315+
posix_kill(posix_getpid(), SIGINT);
2316+
}
2317+
2318+
$output->writeln('Wrapping up, wait a sec...');
2319+
2320+
return 51;
2321+
}
2322+
2323+
public function getSubscribedSignals(): array
2324+
{
2325+
return [\SIGINT];
2326+
}
2327+
2328+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2329+
{
2330+
$this->shouldContinue = false;
2331+
2332+
$this->output->writeln(json_encode(['exit code', $signal, $previousExitCode]));
2333+
2334+
return false;
2335+
}
2336+
2337+
public function handleSignalEvent(ConsoleSignalEvent $event): void
2338+
{
2339+
$this->output->writeln(json_encode(['handling event', $event->getHandlingSignal(), $event->getExitCode()]));
2340+
2341+
$event->setExitCode(125);
2342+
}
2343+
2344+
public static function getSubscribedEvents(): array
2345+
{
2346+
return [
2347+
ConsoleEvents::SIGNAL => 'handleSignalEvent',
2348+
];
22392349
}
22402350
}
22412351

@@ -2248,6 +2358,8 @@ public function onSignal(ConsoleSignalEvent $event): void
22482358
$this->signaled = true;
22492359
$event->getCommand()->signaled = true;
22502360
$event->getCommand()->signalHandlers[] = __CLASS__;
2361+
2362+
$event->abortExit();
22512363
}
22522364

22532365
public static function getSubscribedEvents(): array

‎src/Symfony/Component/Console/Tests/ConsoleEventsTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Tests/ConsoleEventsTest.php
+14Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,20 @@
3030

3131
class ConsoleEventsTest extends TestCase
3232
{
33+
protected function tearDown(): void
34+
{
35+
if (\function_exists('pcntl_signal')) {
36+
pcntl_async_signals(false);
37+
// We reset all signals to their default value to avoid side effects
38+
for ($i = 1; $i <= 15; ++$i) {
39+
if (9 === $i) {
40+
continue;
41+
}
42+
pcntl_signal($i, SIG_DFL);
43+
}
44+
}
45+
}
46+
3347
public function testEventAliases()
3448
{
3549
$container = new ContainerBuilder();

‎src/Symfony/Component/Console/Tests/Fixtures/application_signalable.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Tests/Fixtures/application_signalable.php
+2-4Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
<?php
22

3-
use Symfony\Component\Console\Application;
43
use Symfony\Component\Console\Command\SignalableCommandInterface;
5-
use Symfony\Component\Console\Helper\QuestionHelper;
64
use Symfony\Component\Console\Input\InputInterface;
75
use Symfony\Component\Console\Output\OutputInterface;
86
use Symfony\Component\Console\Question\ChoiceQuestion;
@@ -20,9 +18,9 @@ public function getSubscribedSignals(): array
2018
return [SIGINT];
2119
}
2220

23-
public function handleSignal(int $signal): void
21+
public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
2422
{
25-
exit;
23+
exit(0);
2624
}
2725
})
2826
->setCode(function(InputInterface $input, OutputInterface $output) {

‎src/Symfony/Component/Console/Tests/SignalRegistry/SignalRegistryTest.php

Copy file name to clipboardExpand all lines: src/Symfony/Component/Console/Tests/SignalRegistry/SignalRegistryTest.php
+7-2Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,13 @@ class SignalRegistryTest extends TestCase
2222
protected function tearDown(): void
2323
{
2424
pcntl_async_signals(false);
25-
pcntl_signal(\SIGUSR1, \SIG_DFL);
26-
pcntl_signal(\SIGUSR2, \SIG_DFL);
25+
// We reset all signals to their default value to avoid side effects
26+
for ($i = 1; $i <= 15; ++$i) {
27+
if (9 === $i) {
28+
continue;
29+
}
30+
pcntl_signal($i, SIG_DFL);
31+
}
2732
}
2833

2934
public function testOneCallbackForASignalSignalIsHandled()

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.