symfony · nicolas-grekas · Mar 3, 2023 · Feb 24, 2023
diff --git a/.github/expected-missing-return-types.diff b/.github/expected-missing-return-types.diff
diff --git a/UPGRADE-6.3.md b/UPGRADE-6.3.md
@@ -1,6 +1,12 @@
 UPGRADE FROM 6.2 to 6.3
 =======================
 
+Console
+-------
+
+ * Return int or false from `SignalableCommandInterface::handleSignal()` instead
+   of void and add a second argument `$previousExitCode`
+
 DependencyInjection
 -------------------
 

@@ -1000,37 +1000,62 @@ protected function doRunCommand(Command $command, InputInterface $input, OutputI
             }
         }
 
-        if ($this->signalsToDispatchEvent) {
-            $commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : [];
-
-            if ($commandSignals || null !== $this->dispatcher) {
-                if (!$this->signalRegistry) {
-                    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.');
-                }
+        $commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : [];
+        if ($commandSignals || $this->dispatcher && $this->signalsToDispatchEvent) {
+            if (!$this->signalRegistry) {
+                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.');
+            }
 
-                if (Terminal::hasSttyAvailable()) {
-                    $sttyMode = shell_exec('stty -g');
+            if (Terminal::hasSttyAvailable()) {
+                $sttyMode = shell_exec('stty -g');
 
-                    foreach ([\SIGINT, \SIGTERM] as $signal) {
-                        $this->signalRegistry->register($signal, static function () use ($sttyMode) {
-                            shell_exec('stty '.$sttyMode);
-                        });
-                    }
+                foreach ([\SIGINT, \SIGTERM] as $signal) {
+                    $this->signalRegistry->register($signal, static fn () => shell_exec('stty '.$sttyMode));
                 }
             }
 
-            if (null !== $this->dispatcher) {
+            if ($this->dispatcher) {
+                // We register application signals, so that we can dispatch the event
                 foreach ($this->signalsToDispatchEvent as $signal) {
                     $event = new ConsoleSignalEvent($command, $input, $output, $signal);
 
-                    $this->signalRegistry->register($signal, function () use ($event) {
+                    $this->signalRegistry->register($signal, function ($signal) use ($event, $command, $commandSignals) {
                         $this->dispatcher->dispatch($event, ConsoleEvents::SIGNAL);
+                        $exitCode = $event->getExitCode();
+
+                        // If the command is signalable, we call the handleSignal() method
+                        if (\in_array($signal, $commandSignals, true)) {
+                            $exitCode = $command->handleSignal($signal, $exitCode);
+                            // BC layer for Symfony <= 5
+                            if (null === $exitCode) {
+                                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));
+                                $exitCode = 0;
+                            }
+                        }
+
+                        if (false !== $exitCode) {
+                            exit($exitCode);
+                        }
                     });
                 }
+
+                // then we register command signals, but not if already handled after the dispatcher
+                $commandSignals = array_diff($commandSignals, $this->signalsToDispatchEvent);
             }
 
             foreach ($commandSignals as $signal) {
-                $this->signalRegistry->register($signal, [$command, 'handleSignal']);
+                $this->signalRegistry->register($signal, function (int $signal) use ($command): void {
+                    $exitCode = $command->handleSignal($signal);
+                    // BC layer for Symfony <= 5
+                    if (null === $exitCode) {
+                        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));
+                        $exitCode = 0;
+                    }
+
+                    if (false !== $exitCode) {
+                        exit($exitCode);
+                    }
+                });
             }
         }
 

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

@@ -25,6 +25,10 @@ public function getSubscribedSignals(): array;
 
     /**
      * The method will be called when the application is signaled.
+     *
+     * @param int|false $previousExitCode
+
+     * @return int|false The exit code to return or false to continue the normal execution
      */
-    public function handleSignal(int $signal): void;
+    public function handleSignal(int $signal, /* int|false $previousExitCode = 0 */);
 }
@@ -21,15 +21,36 @@
 final class ConsoleSignalEvent extends ConsoleEvent
 {
     private int $handlingSignal;
+    private int|false $exitCode;
 
-    public function __construct(Command $command, InputInterface $input, OutputInterface $output, int $handlingSignal)
+    public function __construct(Command $command, InputInterface $input, OutputInterface $output, int $handlingSignal, int|false $exitCode = 0)
     {
         parent::__construct($command, $input, $output);
         $this->handlingSignal = $handlingSignal;
+        $this->exitCode = $exitCode;
     }
 
     public function getHandlingSignal(): int
     {
         return $this->handlingSignal;
     }
+
+    public function setExitCode(int $exitCode): void
+    {
+        if ($exitCode < 0 || $exitCode > 255) {
+            throw new \InvalidArgumentException('Exit code must be between 0 and 255.');
+        }
+
+        $this->exitCode = $exitCode;
+    }
+
+    public function abortExit(): void
+    {
+        $this->exitCode = false;
+    }
+
+    public function getExitCode(): int|false
+    {
+        return $this->exitCode;
+    }
 }
@@ -20,6 +20,7 @@
 use Symfony\Component\Console\Command\SignalableCommandInterface;
 use Symfony\Component\Console\CommandLoader\CommandLoaderInterface;
 use Symfony\Component\Console\CommandLoader\FactoryCommandLoader;
+use Symfony\Component\Console\ConsoleEvents;
 use Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
 use Symfony\Component\Console\Event\ConsoleErrorEvent;
@@ -1929,7 +1930,8 @@ public function testSignalListener()
 
         $dispatcherCalled = false;
         $dispatcher = new EventDispatcher();
-        $dispatcher->addListener('console.signal', function () use (&$dispatcherCalled) {
+        $dispatcher->addListener('console.signal', function (ConsoleSignalEvent $e) use (&$dispatcherCalled) {
+            $e->abortExit();
             $dispatcherCalled = true;
         });
 
@@ -1978,6 +1980,34 @@ public function testSignalSubscriber()
         $this->assertTrue($subscriber2->signaled);
     }
 
+    /**
+     * @requires extension pcntl
+     */
+    public function testSignalDispatchWithoutEventToDispatch()
+    {
+        $command = new SignableCommand();
+
+        $application = $this->createSignalableApplication($command, null);
+        $application->setSignalsToDispatchEvent();
+
+        $this->assertSame(1, $application->run(new ArrayInput(['signal'])));
+        $this->assertTrue($command->signaled);
+    }
+
+    /**
+     * @requires extension pcntl
+     */
+    public function testSignalDispatchWithoutEventDispatcher()
+    {
+        $command = new SignableCommand();
+
+        $application = $this->createSignalableApplication($command, null);
+        $application->setSignalsToDispatchEvent(\SIGUSR1);
+
+        $this->assertSame(1, $application->run(new ArrayInput(['signal'])));
+        $this->assertTrue($command->signaled);
+    }
+
     /**
      * @requires extension pcntl
      */
@@ -2077,9 +2107,36 @@ public function testSignalableCommandDoesNotInterruptedOnTermSignals()
         $application->setAutoExit(false);
         $application->setDispatcher($dispatcher);
         $application->add($command);
+
         $this->assertSame(129, $application->run(new ArrayInput(['signal'])));
     }
 
+    public function testSignalableWithEventCommandDoesNotInterruptedOnTermSignals()
+    {
+        if (!\defined('SIGINT')) {
+            $this->markTestSkipped('SIGINT not available');
+        }
+
+        $command = new TerminatableWithEventCommand();
+
+        $dispatcher = new EventDispatcher();
+        $dispatcher->addSubscriber($command);
+        $application = new Application();
+        $application->setAutoExit(false);
+        $application->setDispatcher($dispatcher);
+        $application->add($command);
+        $tester = new ApplicationTester($application);
+        $this->assertSame(51, $tester->run(['signal']));
+        $expected = <<<EOTXT
+            Still processing...
+            ["handling event",2,0]
+            ["exit code",2,125]
+            Wrapping up, wait a sec...
+
+            EOTXT;
+        $this->assertSame($expected, $tester->getDisplay(true));
+    }
+
     /**
      * @group tty
      */
@@ -2217,10 +2274,12 @@ public function getSubscribedSignals(): array
         return SignalRegistry::isSupported() ? [\SIGUSR1] : [];
     }
 
-    public function handleSignal(int $signal): void
+    public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
     {
         $this->signaled = true;
         $this->signalHandlers[] = __CLASS__;
+
+        return false;
     }
 }
 
@@ -2232,10 +2291,61 @@ public function getSubscribedSignals(): array
         return SignalRegistry::isSupported() ? [\SIGINT] : [];
     }
 
-    public function handleSignal(int $signal): void
+    public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
     {
         $this->signaled = true;
         $this->signalHandlers[] = __CLASS__;
+
+        return false;
+    }
+}
+
+#[AsCommand(name: 'signal')]
+class TerminatableWithEventCommand extends Command implements SignalableCommandInterface, EventSubscriberInterface
+{
+    private bool $shouldContinue = true;
+    private OutputInterface $output;
+
+    protected function execute(InputInterface $input, OutputInterface $output): int
+    {
+        $this->output = $output;
+
+        for ($i = 0; $i <= 10 && $this->shouldContinue; ++$i) {
+            $output->writeln('Still processing...');
+            posix_kill(posix_getpid(), SIGINT);
+        }
+
+        $output->writeln('Wrapping up, wait a sec...');
+
+        return 51;
+    }
+
+    public function getSubscribedSignals(): array
+    {
+        return [\SIGINT];
+    }
+
+    public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
+    {
+        $this->shouldContinue = false;
+
+        $this->output->writeln(json_encode(['exit code', $signal, $previousExitCode]));
+
+        return false;
+    }
+
+    public function handleSignalEvent(ConsoleSignalEvent $event): void
+    {
+        $this->output->writeln(json_encode(['handling event', $event->getHandlingSignal(), $event->getExitCode()]));
+
+        $event->setExitCode(125);
+    }
+
+    public static function getSubscribedEvents(): array
+    {
+        return [
+            ConsoleEvents::SIGNAL => 'handleSignalEvent',
+        ];
     }
 }
 
@@ -2248,6 +2358,8 @@ public function onSignal(ConsoleSignalEvent $event): void
         $this->signaled = true;
         $event->getCommand()->signaled = true;
         $event->getCommand()->signalHandlers[] = __CLASS__;
+
+        $event->abortExit();
     }
 
     public static function getSubscribedEvents(): array

@@ -30,6 +30,20 @@
 
 class ConsoleEventsTest extends TestCase
 {
+    protected function tearDown(): void
+    {
+        if (\function_exists('pcntl_signal')) {
+            pcntl_async_signals(false);
+            // We reset all signals to their default value to avoid side effects
+            for ($i = 1; $i <= 15; ++$i) {
+                if (9 === $i) {
+                    continue;
+                }
+                pcntl_signal($i, SIG_DFL);
+            }
+        }
+    }
+
     public function testEventAliases()
     {
         $container = new ContainerBuilder();

@@ -1,8 +1,6 @@
 <?php
 
-use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Command\SignalableCommandInterface;
-use Symfony\Component\Console\Helper\QuestionHelper;
 use Symfony\Component\Console\Input\InputInterface;
 use Symfony\Component\Console\Output\OutputInterface;
 use Symfony\Component\Console\Question\ChoiceQuestion;
@@ -20,9 +18,9 @@ public function getSubscribedSignals(): array
         return [SIGINT];
     }
 
-    public function handleSignal(int $signal): void
+    public function handleSignal(int $signal, int|false $previousExitCode = 0): int|false
     {
-        exit;
+        exit(0);
     }
 })
     ->setCode(function(InputInterface $input, OutputInterface $output) {

@@ -22,8 +22,13 @@ class SignalRegistryTest extends TestCase
     protected function tearDown(): void
     {
         pcntl_async_signals(false);
-        pcntl_signal(\SIGUSR1, \SIG_DFL);
-        pcntl_signal(\SIGUSR2, \SIG_DFL);
+        // We reset all signals to their default value to avoid side effects
+        for ($i = 1; $i <= 15; ++$i) {
+            if (9 === $i) {
+                continue;
+            }
+            pcntl_signal($i, SIG_DFL);
+        }
  
4FD5
   }
 
     public function testOneCallbackForASignalSignalIsHandled()