From 37b1faec8c8f2ad29591fcba5297c5f99e401e2b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gr=C3=A9goire=20Pineau?= <lyrixx@lyrixx.info>
Date: Tue, 27 Oct 2020 16:32:21 +0100
Subject: [PATCH] [Console] Register signal handling only for commands
 implemeting SignalableCommandInterface

Actually, it does not make sens to listen all signals for all commands.

This commit also add more test for this part of code.
---
 src/Symfony/Component/Console/Application.php | 35 +++++-----
 .../Console/Tests/ApplicationTest.php         | 64 +++++++++++++++++++
 2 files changed, 82 insertions(+), 17 deletions(-)

diff --git a/src/Symfony/Component/Console/Application.php b/src/Symfony/Component/Console/Application.php
index 0efccdb103..cdb5a81d25 100644
--- a/src/Symfony/Component/Console/Application.php
+++ b/src/Symfony/Component/Console/Application.php
@@ -286,23 +286,6 @@ class Application implements ResetInterface
             $command = $this->find($alternative);
         }
 
-        if ($this->dispatcher && $this->signalRegistry) {
-            foreach ($this->signalsToDispatchEvent as $signal) {
-                $event = new ConsoleSignalEvent($command, $input, $output, $signal);
-
-                $this->signalRegistry->register($signal, function ($signal, $hasNext) use ($event) {
-                    $this->dispatcher->dispatch($event, ConsoleEvents::SIGNAL);
-
-                    // No more handlers, we try to simulate PHP default behavior
-                    if (!$hasNext) {
-                        if (!\in_array($signal, [\SIGUSR1, \SIGUSR2], true)) {
-                            exit(0);
-                        }
-                    }
-                });
-            }
-        }
-
         $this->runningCommand = $command;
         $exitCode = $this->doRunCommand($command, $input, $output);
         $this->runningCommand = null;
@@ -961,6 +944,24 @@ class Application implements ResetInterface
             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 ($this->dispatcher) {
+                foreach ($this->signalsToDispatchEvent as $signal) {
+                    $event = new ConsoleSignalEvent($command, $input, $output, $signal);
+
+                    $this->signalRegistry->register($signal, function ($signal, $hasNext) use ($event) {
+                        $this->dispatcher->dispatch($event, ConsoleEvents::SIGNAL);
+
+                        // No more handlers, we try to simulate PHP default behavior
+                        if (!$hasNext) {
+                            if (!\in_array($signal, [\SIGUSR1, \SIGUSR2], true)) {
+                                exit(0);
+                            }
+                        }
+                    });
+                }
+            }
+
             foreach ($command->getSubscribedSignals() as $signal) {
                 $this->signalRegistry->register($signal, [$command, 'handleSignal']);
             }
diff --git a/src/Symfony/Component/Console/Tests/ApplicationTest.php b/src/Symfony/Component/Console/Tests/ApplicationTest.php
index 88631e0b6e..4141920965 100644
--- a/src/Symfony/Component/Console/Tests/ApplicationTest.php
+++ b/src/Symfony/Component/Console/Tests/ApplicationTest.php
@@ -14,6 +14,7 @@ namespace Symfony\Component\Console\Tests;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Console\Application;
 use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Command\SignalableCommandInterface;
 use Symfony\Component\Console\CommandLoader\FactoryCommandLoader;
 use Symfony\Component\Console\DependencyInjection\AddConsoleCommandPass;
 use Symfony\Component\Console\Event\ConsoleCommandEvent;
@@ -1808,6 +1809,39 @@ class ApplicationTest extends TestCase
         $app->setCommandLoader($loader);
         $app->get('test');
     }
+
+    /**
+     * @requires extension pcntl
+     */
+    public function testSignal()
+    {
+        $command = new SignableCommand();
+
+        $dispatcherCalled = false;
+        $dispatcher = new EventDispatcher();
+        $dispatcher->addListener('console.signal', function () use (&$dispatcherCalled) {
+            $dispatcherCalled = true;
+        });
+
+        $application = new Application();
+        $application->setAutoExit(false);
+        $application->setDispatcher($dispatcher);
+        $application->setSignalsToDispatchEvent(SIGALRM);
+        $application->add($command);
+
+        $this->assertFalse($command->signaled);
+        $this->assertFalse($dispatcherCalled);
+
+        $this->assertSame(0, $application->run(new ArrayInput(['signal'])));
+        $this->assertFalse($command->signaled);
+        $this->assertFalse($dispatcherCalled);
+
+        $command->loop = 100000;
+        pcntl_alarm(1);
+        $this->assertSame(1, $application->run(new ArrayInput(['signal'])));
+        $this->assertTrue($command->signaled);
+        $this->assertTrue($dispatcherCalled);
+    }
 }
 
 class CustomApplication extends Application
@@ -1865,3 +1899,33 @@ class DisabledCommand extends Command
         return false;
     }
 }
+
+class SignableCommand extends Command implements SignalableCommandInterface
+{
+    public $signaled = false;
+    public $loop = 100;
+
+    protected static $defaultName = 'signal';
+
+    public function getSubscribedSignals(): array
+    {
+        return [SIGALRM];
+    }
+
+    public function handleSignal(int $signal): void
+    {
+        $this->signaled = true;
+    }
+
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        for ($i = 0; $i < $this->loop; ++$i) {
+            usleep(100);
+            if ($this->signaled) {
+                return 1;
+            }
+        }
+
+        return 0;
+    }
+}