From 6e5dbeadd67244baf7c7c870a6687e0f58e37d33 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Fri, 29 Nov 2019 23:50:07 +0100 Subject: [PATCH 01/16] Add phpstan and remove unsupported class --- src/Spork/EventDispatcher/EventDispatcher.php | 29 ++--- .../EventDispatcherInterface.php | 52 +++++++-- .../EventDispatcher/EventDispatcherTrait.php | 109 ++++++++++++++++++ src/Spork/EventDispatcher/SignalEvent.php | 103 +++++++++++++++++ .../WrappedEventDispatcher.php | 79 ++++++++----- tests/Spork/SignalTest.php | 46 +++++++- 6 files changed, 357 insertions(+), 61 deletions(-) create mode 100644 src/Spork/EventDispatcher/EventDispatcherTrait.php create mode 100644 src/Spork/EventDispatcher/SignalEvent.php diff --git a/src/Spork/EventDispatcher/EventDispatcher.php b/src/Spork/EventDispatcher/EventDispatcher.php index 93b7c20..500b518 100644 --- a/src/Spork/EventDispatcher/EventDispatcher.php +++ b/src/Spork/EventDispatcher/EventDispatcher.php @@ -1,38 +1,25 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ -declare(ticks=1); +declare(strict_types=1); namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcher as BaseEventDispatcher; +use Symfony\Component\EventDispatcher\EventDispatcher as BaseClass; /** - * Adds support for listening to signals. + * Extends the core event dispatcher with signal handling capabilities. Add and + * remove signal listeners or dispatch a signal directly. */ -class EventDispatcher extends BaseEventDispatcher implements EventDispatcherInterface +class EventDispatcher extends BaseClass implements EventDispatcherInterface { - public function dispatchSignal($signal) - { - $this->dispatch('spork.signal.' . $signal); - } - - public function addSignalListener($signal, $callable, $priority = 0) - { - $this->addListener('spork.signal.' . $signal, $callable, $priority); - pcntl_signal($signal, [$this, 'dispatchSignal']); - } - - public function removeSignalListener($signal, $callable) - { - $this->removeListener('spork.signal.' . $signal, $callable); - } + use EventDispatcherTrait; } diff --git a/src/Spork/EventDispatcher/EventDispatcherInterface.php b/src/Spork/EventDispatcher/EventDispatcherInterface.php index 6f8e77b..87e2242 100644 --- a/src/Spork/EventDispatcher/EventDispatcherInterface.php +++ b/src/Spork/EventDispatcher/EventDispatcherInterface.php @@ -1,21 +1,59 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseEventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseInterface; -interface EventDispatcherInterface extends BaseEventDispatcherInterface +/** + * Extends the core event dispatcher interface with signal handling + * capabilities. Add and remove signal listeners or dispatch a signal directly. + */ +interface EventDispatcherInterface extends BaseInterface { - public function dispatchSignal($signal); - public function addSignalListener($signal, $callable, $priority = 0); - public function removeSignalListener($signal, $callable); + /** + * The signal handler. + * + * @param int $signo The signal being handled. + * @param mixed $signinfo If operating systems supports siginfo_t + * structures, this will be an array of signal + * information dependent on the signal. + * @return \Spork\EventDispatcher\SignalEvent Holds the signal information. + */ + public function dispatchSignal(int $signo, $signinfo): SignalEvent; + + /** + * Adds a signal listener that listens on the specified signal. + * + * @param int $signo The signal number. + * @param callable $listener The listener. + * @param int $priority The higher this value, the earlier an event + * listener will be triggered in the chain + * (defaults to 0) + * @return void + */ + public function addSignalListener( + int $signo, + callable $listener, + int $priority = 0 + ): void; + + /** + * Removes a signal listener from the specified signal. + * + * @param int $signo The signal to remove a listener from. + * @param callable $listener The listener to remove. + * @return void + */ + public function removeSignalListener(int $signo, callable $listener): void; } diff --git a/src/Spork/EventDispatcher/EventDispatcherTrait.php b/src/Spork/EventDispatcher/EventDispatcherTrait.php new file mode 100644 index 0000000..b07205b --- /dev/null +++ b/src/Spork/EventDispatcher/EventDispatcherTrait.php @@ -0,0 +1,109 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\EventDispatcher; + +use UnexpectedValueException; + +/** + * Common implementation of the signal related EventDispatcherInterface interface. + */ +trait EventDispatcherTrait +{ + /** + * Holds previous signal handlers, which will be restored when a signal + * handler is removed. + * + * @var array $prevSigHandlers + */ + private $prevSigHandlers = []; + + /** + * {@inheritDoc} + */ + public function dispatchSignal(int $signo, $signinfo): SignalEvent + { + $event = new SignalEvent($signo, $signinfo); + + return $this->dispatch($event, $event->getName()); + } + + /** + * {@inheritDoc} + */ + public function addSignalListener( + int $signo, + callable $listener, + int $priority = 0 + ): void { + // 1. Preserve previous signal handler. + $prevSigHandler = pcntl_signal_get_handler($signo); + if ($prevSigHandler === false) { + $error = pcntl_get_last_error(); + if ($error === 0) { + $error = PCNTL_EINVAL; + } + $strerror = pcntl_strerror($error); + + throw new UnexpectedValueException(sprintf( + 'Could not get currently installed signal handler for signal ' . + '%d. %d: %s', + $signo, + $error, + $strerror + )); + } + $this->prevSigHandlers[$signo] = $prevSigHandler; + + // 2. Install our new signal handler. + $newSigHandler = [$this, 'dispatchSignal']; + if (is_callable($prevSigHandler)) { + $newSigHandler = function ( + int $signo, + $signinfo + ) use (&$prevSigHandler) { + $prevSigHandler($signo, $signinfo); + call_user_func([$this, 'dispatchSignal'], $signo, $signinfo); + }; + } + if (!pcntl_signal($signo, $newSigHandler)) { + $error = pcntl_get_last_error(); + if ($error === 0) { + $error = PCNTL_EINVAL; + } + $strerror = pcntl_strerror($error); + + throw new UnexpectedValueException(sprintf( + 'Could not install signal handler for signal %d. %d: %s', + $signo, + $error, + $strerror + )); + } + + // 3. Add listener to event. + $this->addListener( + SignalEvent::getEventName($signo), + $listener, + $priority + ); + } + + /** + * {@inheritDoc} + */ + public function removeSignalListener(int $signo, callable $listener): void + { + $this->removeListener(SignalEvent::getEventName($signo), $listener); + } +} diff --git a/src/Spork/EventDispatcher/SignalEvent.php b/src/Spork/EventDispatcher/SignalEvent.php new file mode 100644 index 0000000..3651297 --- /dev/null +++ b/src/Spork/EventDispatcher/SignalEvent.php @@ -0,0 +1,103 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\EventDispatcher; + +use Symfony\Contracts\EventDispatcher\Event as BaseEvent; + +/** + * Holds the signal information. + */ +class SignalEvent extends BaseEvent +{ + /** + * Event prefix used to construct the full event name, which contains the + * signal number. + * + * @var string PREFIX + */ + private const PREFIX = 'spork.signal.'; + + /** + * The signal being handled. + * + * @var int $signo + */ + private $signo; + + /** + * If operating systems supports siginfo_t structures, this will be an array + * of signal information dependent on the signal. + * + * @var mixed + */ + private $signinfo; + + /** + * Gets the event name for a signal number. + * + * @param int $signo A signal number. + * @return string The event name for a signal number. + */ + public static function getEventName(int $signo): string + { + return self::PREFIX . $signo; + } + + /** + * Constructs a new instance of the Event class. + * + * @param int $signo The signal being handled. + * @param mixed $signinfo If operating systems supports siginfo_t + * structures, this will be an array of signal + * information dependent on the signal. + */ + public function __construct(int $signo, $signinfo) + { + $this->signo = $signo; + $this->signinfo = $signinfo; + } + + /** + * Gets the signal being handled. + * + * @return int The signal being handled. + */ + public function getSigno(): int + { + return $this->signo; + } + + /** + * If operating systems supports siginfo_t structures, this will get the + * array of signal information dependent on the signal. + * + * @return mixed If operating systems supports siginfo_t structures, this + * will be an array of signal information dependent on the + * signal. + */ + public function getSigninfo() + { + return $this->signinfo; + } + + /** + * Gets the event name of the signal being handled. + * + * @return string The event name of the signal being handled. + */ + public function getName(): string + { + return static::getEventName($this->getSigno()); + } +} diff --git a/src/Spork/EventDispatcher/WrappedEventDispatcher.php b/src/Spork/EventDispatcher/WrappedEventDispatcher.php index 3636b23..ebc27a9 100644 --- a/src/Spork/EventDispatcher/WrappedEventDispatcher.php +++ b/src/Spork/EventDispatcher/WrappedEventDispatcher.php @@ -1,82 +1,105 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ -declare(ticks=1); +declare(strict_types=1); namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\Event; -use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseEventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; +/** + * Wraps another event dispatcher, adding signal handling capabilities to it. + */ class WrappedEventDispatcher implements EventDispatcherInterface { + use EventDispatcherTrait; + + /** + * The wrapped event dispatcher. + * + * @var \Symfony\Component\EventDispatcher\EventDispatcherInterface $delegate + */ private $delegate; - public function __construct(BaseEventDispatcherInterface $delegate) + /** + * Constructs a new instance of the WrappedEventDispatcher class. + * + * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $delegate + * The wrapped event dispatcher. + */ + public function __construct(BaseInterface $delegate) { $this->delegate = $delegate; } - public function dispatchSignal($signal) - { - $this->delegate->dispatch('spork.signal.' . $signal); - } - - public function addSignalListener($signal, $callable, $priority = 0) - { - $this->delegate->addListener('spork.signal.' . $signal, $callable, $priority); - pcntl_signal($signal, [$this, 'dispatchSignal']); - } - - public function removeSignalListener($signal, $callable) - { - $this->delegate->removeListener('spork.signal.' . $signal, $callable); - } - - public function dispatch($eventName, Event $event = null) + /** + * {@inheritDoc} + */ + public function dispatch($event, string $eventName = null) { - return $this->delegate->dispatch($eventName, $event); + return $this->delegate->dispatch($event, $eventName); } + /** + * {@inheritDoc} + */ public function addListener($eventName, $listener, $priority = 0) { - $this->delegate->addListener($eventName, $listener, $priority); + return $this->delegate->addListener($eventName, $listener, $priority); } + /** + * {@inheritDoc} + */ public function addSubscriber(EventSubscriberInterface $subscriber) { - $this->delegate->addSubscriber($subscriber); + return $this->delegate->addSubscriber($subscriber); } + /** + * {@inheritDoc} + */ public function removeListener($eventName, $listener) { - $this->delegate->removeListener($eventName, $listener); + return $this->delegate->removeListener($eventName, $listener); } + /** + * {@inheritDoc} + */ public function removeSubscriber(EventSubscriberInterface $subscriber) { - $this->delegate->removeSubscriber($subscriber); + return $this->delegate->removeSubscriber($subscriber); } + /** + * {@inheritDoc} + */ public function getListeners($eventName = null) { return $this->delegate->getListeners($eventName); } + /** + * {@inheritDoc} + */ public function getListenerPriority($eventName, $listener) { return $this->delegate->getListenerPriority($eventName, $listener); } + /** + * {@inheritDoc} + */ public function hasListeners($eventName = null) { return $this->delegate->hasListeners($eventName); diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index aae25b6..4dd30e3 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -1,17 +1,21 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork; use PHPUnit\Framework\TestCase; +use Spork\EventDispatcher\EventDispatcher; +use Spork\EventDispatcher\SignalEvent; class SignalTest extends TestCase { @@ -21,8 +25,12 @@ class SignalTest extends TestCase /** @var bool $async */ private $async; + /** @var int $errorReporting */ + private $errorReporting; + protected function setUp(): void { + $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); $this->async = pcntl_async_signals(); pcntl_async_signals(true); @@ -34,15 +42,43 @@ protected function tearDown(): void $this->manager = null; pcntl_async_signals($this->async); + $this->errorReporting = error_reporting($this->errorReporting); } public function testSignalParent() { $signaled = false; - $this->manager->addListener(SIGUSR1, function () use (&$signaled) { - $signaled = true; - }); + $this->manager->addListener( + SIGUSR1, + function ( + SignalEvent $event, + string $eventName, + EventDispatcher $dispatcher + ) use (&$signaled) { + $signaled = true; + + $this->assertEquals(SIGUSR1, $event->getSigno()); + + $signoInfo = $event->getSigninfo(); + if (is_array($signoInfo)) { + $this->assertIsArray($signoInfo); + $this->assertArrayHasKey('signo', $signoInfo); + $this->assertEquals(SIGUSR1, $signoInfo['signo']); + $this->assertArrayHasKey('errno', $signoInfo); + $this->assertIsInt($signoInfo['errno']); + $this->assertArrayHasKey('code', $signoInfo); + $this->assertIsInt($signoInfo['code']); + $this->assertArrayHasKey('pid', $signoInfo); + $this->assertIsInt($signoInfo['pid']); + $this->assertArrayHasKey('uid', $signoInfo); + $this->assertIsInt($signoInfo['uid']); + } + + $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); + $this->assertEquals($this->manager->getEventDispatcher(), $dispatcher); + } + ); $this->manager->fork(function (SharedMemory $sharedMem) { $sharedMem->signal(SIGUSR1); From 5c590b7e64c019daa267ffc45861a41885f3051b Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Fri, 13 Dec 2019 18:32:50 +0100 Subject: [PATCH 02/16] #5: Fix phpstan errors --- src/Spork/EventDispatcher/EventDispatcherTrait.php | 5 ++++- src/Spork/EventDispatcher/WrappedEventDispatcher.php | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Spork/EventDispatcher/EventDispatcherTrait.php b/src/Spork/EventDispatcher/EventDispatcherTrait.php index b07205b..13146a3 100644 --- a/src/Spork/EventDispatcher/EventDispatcherTrait.php +++ b/src/Spork/EventDispatcher/EventDispatcherTrait.php @@ -35,7 +35,9 @@ public function dispatchSignal(int $signo, $signinfo): SignalEvent { $event = new SignalEvent($signo, $signinfo); - return $this->dispatch($event, $event->getName()); + $this->dispatch($event, $event->getName()); + + return $event; } /** @@ -47,6 +49,7 @@ public function addSignalListener( int $priority = 0 ): void { // 1. Preserve previous signal handler. + /** @var int|string|false $prevSigHandler */ $prevSigHandler = pcntl_signal_get_handler($signo); if ($prevSigHandler === false) { $error = pcntl_get_last_error(); diff --git a/src/Spork/EventDispatcher/WrappedEventDispatcher.php b/src/Spork/EventDispatcher/WrappedEventDispatcher.php index ebc27a9..b54f44f 100644 --- a/src/Spork/EventDispatcher/WrappedEventDispatcher.php +++ b/src/Spork/EventDispatcher/WrappedEventDispatcher.php @@ -46,7 +46,7 @@ public function __construct(BaseInterface $delegate) */ public function dispatch($event, string $eventName = null) { - return $this->delegate->dispatch($event, $eventName); + return call_user_func([$this->delegate, 'dispatch'], $event, $eventName); } /** From e1518e3b7a0d3180374075782159099c8062c0fb Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Wed, 25 Dec 2019 19:14:54 +0100 Subject: [PATCH 03/16] #5: Restore previous signal handler on desutruction --- composer.json | 3 +- .../EventDispatcher/EventDispatcherTrait.php | 145 ++++++++++----- src/Spork/Signal/SignalHandlerWrapper.php | 115 ++++++++++++ tests/Spork/SignalTest.php | 166 +++++++++++++++--- 4 files changed, 366 insertions(+), 63 deletions(-) create mode 100644 src/Spork/Signal/SignalHandlerWrapper.php diff --git a/composer.json b/composer.json index 46704cb..45773d7 100644 --- a/composer.json +++ b/composer.json @@ -45,7 +45,8 @@ "friendsofphp/php-cs-fixer": "^2.16", "phpstan/phpstan": "^0.11.19", "phpunit/phpunit": "^8.4", - "squizlabs/php_codesniffer": "^3.5" + "squizlabs/php_codesniffer": "^3.5", + "symfony/var-dumper": "^4.0.0" }, "autoload": { "psr-4": { diff --git a/src/Spork/EventDispatcher/EventDispatcherTrait.php b/src/Spork/EventDispatcher/EventDispatcherTrait.php index 13146a3..8427e56 100644 --- a/src/Spork/EventDispatcher/EventDispatcherTrait.php +++ b/src/Spork/EventDispatcher/EventDispatcherTrait.php @@ -13,20 +13,22 @@ namespace Spork\EventDispatcher; +use Spork\Signal\SignalHandlerWrapper; use UnexpectedValueException; /** - * Common implementation of the signal related EventDispatcherInterface interface. + * Partial common implementation of the signal related EventDispatcherInterface + * interface. */ trait EventDispatcherTrait { /** - * Holds previous signal handlers, which will be restored when a signal - * handler is removed. + * Holds signal handler wrappers to preserve a potentially already existing + * signal handler. * - * @var array $prevSigHandlers + * @var array<\Spork\Signal\SignalHandlerWrapper> $sigHandlerWrappers */ - private $prevSigHandlers = []; + private $sigHandlerWrappers = []; /** * {@inheritDoc} @@ -48,10 +50,80 @@ public function addSignalListener( callable $listener, int $priority = 0 ): void { - // 1. Preserve previous signal handler. - /** @var int|string|false $prevSigHandler */ - $prevSigHandler = pcntl_signal_get_handler($signo); - if ($prevSigHandler === false) { + $this->checkSignalHandler($signo); + + $this->addListener( + SignalEvent::getEventName($signo), + $listener, + $priority + ); + } + + /** + * {@inheritDoc} + */ + public function removeSignalListener(int $signo, callable $listener): void + { + $this->removeListener(SignalEvent::getEventName($signo), $listener); + } + + public function removeSignalHandlerWrappers(): void + { + foreach ($this->sigHandlerWrappers as $signo => $sigHandlerWrapper) { + /** @var int|callable $prevSigHandler */ + $prevSigHandler = $this->getSignalHandler($signo); + + // Has not been overwritten in the meantime. + if ($prevSigHandler === $sigHandlerWrapper) { + $this->setSignalHandler($signo, $sigHandlerWrapper->getPrevious()); + } + + unset($this->sigHandlerWrappers[$signo]); + } + } + + /** + * Checks whether an event dispatcher signal handler is installed. + * + * @param int $signo The signal being handled. + * @return void + */ + private function checkSignalHandler(int $signo): void + { + if (array_key_exists($signo, $this->sigHandlerWrappers)) { + return; + } + + // 1. Backup previous signal handler. + $this->sigHandlerWrappers[$signo] = new SignalHandlerWrapper( + $this->getSignalHandler($signo), + [$this, 'dispatchSignal'] + ); + + // 2. Install the event dispatcher signal handler. + $this->setSignalHandler($signo, $this->sigHandlerWrappers[$signo]); + } + + /** + * Get the current handler for a specific signal. Throws an exception on + * error. + * + * @param int $signo + * The signal for which to get the current handler. + * @throws \UnexpectedValueException + * When the currently installed signal handler could not be + * retrieved. + * @return int|callable + * Current signal handler. This may be an integer value that + * refers to SIG_DFL or SIG_IGN. If a custom handler was used it + * may be a string value containing the function name, an array + * containing the instance and method name or a callable. + */ + private function getSignalHandler(int $signo) + { + /** @var int|callable|false $signalHandler */ + $signalHandler = pcntl_signal_get_handler($signo); + if ($signalHandler === false) { $error = pcntl_get_last_error(); if ($error === 0) { $error = PCNTL_EINVAL; @@ -59,27 +131,35 @@ public function addSignalListener( $strerror = pcntl_strerror($error); throw new UnexpectedValueException(sprintf( - 'Could not get currently installed signal handler for signal ' . - '%d. %d: %s', + 'Could not get installed signal handler for signal %d. %d: %s', $signo, $error, $strerror )); } - $this->prevSigHandlers[$signo] = $prevSigHandler; - - // 2. Install our new signal handler. - $newSigHandler = [$this, 'dispatchSignal']; - if (is_callable($prevSigHandler)) { - $newSigHandler = function ( - int $signo, - $signinfo - ) use (&$prevSigHandler) { - $prevSigHandler($signo, $signinfo); - call_user_func([$this, 'dispatchSignal'], $signo, $signinfo); - }; - } - if (!pcntl_signal($signo, $newSigHandler)) { + + return $signalHandler; + } + + /** + * Sets a new signal handler for a specific signal. Throws an exception on + * error. + * + * @param int $signo + * When the currently installed signal handler could not be + * retrieved. + * @param int|callable $handler + * Current signal handler. This may be an integer value that + * refers to SIG_DFL or SIG_IGN. If a custom handler was used it + * may be a string value containing the function name, an array + * containing the instance and method name or a callable. + * @throws \UnexpectedValueException + * When the new signal handler could not be set. + * @return void + */ + private function setSignalHandler(int $signo, $handler): void + { + if (!pcntl_signal($signo, $handler)) { $error = pcntl_get_last_error(); if ($error === 0) { $error = PCNTL_EINVAL; @@ -93,20 +173,5 @@ public function addSignalListener( $strerror )); } - - // 3. Add listener to event. - $this->addListener( - SignalEvent::getEventName($signo), - $listener, - $priority - ); - } - - /** - * {@inheritDoc} - */ - public function removeSignalListener(int $signo, callable $listener): void - { - $this->removeListener(SignalEvent::getEventName($signo), $listener); } } diff --git a/src/Spork/Signal/SignalHandlerWrapper.php b/src/Spork/Signal/SignalHandlerWrapper.php new file mode 100644 index 0000000..50448c5 --- /dev/null +++ b/src/Spork/Signal/SignalHandlerWrapper.php @@ -0,0 +1,115 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\Signal; + +/** + * Wraps an existing signal handler with a new signal handler. + */ +class SignalHandlerWrapper +{ + /** + * Previous signal handler. This may be an integer value that refers to + * SIG_DFL or SIG_IGN. If a custom handler was used it may be a string value + * containing the function name, an array containing the instance and method + * name or a callable. + * + * @var int|callable $previous + */ + private $previous; + + /** + * Current signal handler to execute. This may be an integer value that + * refers to SIG_DFL or SIG_IGN. If a custom handler should be used it may + * be a string value containing the function name, an array containing the + * instance and method name or a callable. + * + * @var int|callable $current + */ + private $current; + + /** + * Wrapper callable, which can be used as the new signal handler. It will + * execute the previous and then the new signal handler. + * + * @var callable + */ + private $wrapper; + + /** + * Constructs a new instance of the SignalHandlerWrapper class. + * + * @param int|callable $previous + * @param int|callable $current + */ + public function __construct($previous, $current) + { + $this->previous = $previous; + $this->current = $current; + + $this->wrapper = function (int $signo, $signinfo) { + if (is_callable($this->previous)) { + call_user_func($this->previous, $signo, $signinfo); + } + + if (is_callable($this->current)) { + call_user_func($this->current, $signo, $signinfo); + } + }; + } + + /** + * Invokes the signal handler wrapper. + * + * @param int $signo + * The signal being handled. + * @param mixed $signinfo + * If operating systems supports siginfo_t structures, this will + * be an array of signal information dependent on the signal. + * @return void + */ + public function __invoke(int $signo, $signinfo): void + { + call_user_func($this->wrapper, $signo, $signinfo); + } + + /** + * Gets the previous signal handler. + * + * @return int|callable Previous signal handler. + */ + public function getPrevious() + { + return $this->previous; + } + + /** + * Gets the current signal handler. + * + * @return int|callable Current signal handler. + */ + public function getCurrent() + { + return $this->current; + } + + /** + * Gets the wrapper signal handler. + * + * @return callable Wrapper signal handler. + */ + public function getWrapper(): callable + { + return $this->wrapper; + } +} diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index 8199dbc..13dd5a2 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -19,10 +19,25 @@ class SignalTest extends TestCase { - private $manager; + /** + * Process manager instance. + * + * @var \Spork\ProcessManager $processManager + */ + private $processManager; + + /** + * Holds the previous pcntl async signals value. + * + * @var bool $async + */ private $async; - /** @var int $errorReporting */ + /** + * Holds the previous error reporting configuration. + * + * @var int $errorReporting + */ private $errorReporting; protected function setUp(): void @@ -31,22 +46,22 @@ protected function setUp(): void $this->async = pcntl_async_signals(); pcntl_async_signals(true); - $this->manager = new ProcessManager(); + $this->processManager = new ProcessManager(); } protected function tearDown(): void { - $this->manager = null; + $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); pcntl_async_signals($this->async); $this->errorReporting = error_reporting($this->errorReporting); } - public function testSignalParent() + public function testSingleListenerOneSignal() { $signaled = false; - $this->manager->addListener( + $this->processManager->addListener( SIGUSR1, function ( SignalEvent $event, @@ -57,32 +72,139 @@ function ( $this->assertEquals(SIGUSR1, $event->getSigno()); - $signoInfo = $event->getSigninfo(); - if (is_array($signoInfo)) { - $this->assertIsArray($signoInfo); - $this->assertArrayHasKey('signo', $signoInfo); - $this->assertEquals(SIGUSR1, $signoInfo['signo']); - $this->assertArrayHasKey('errno', $signoInfo); - $this->assertIsInt($signoInfo['errno']); - $this->assertArrayHasKey('code', $signoInfo); - $this->assertIsInt($signoInfo['code']); - $this->assertArrayHasKey('pid', $signoInfo); - $this->assertIsInt($signoInfo['pid']); - $this->assertArrayHasKey('uid', $signoInfo); - $this->assertIsInt($signoInfo['uid']); + $signinfo = $event->getSigninfo(); + if (is_array($signinfo)) { + $this->assertArrayHasKey('signo', $signinfo); + if ($signinfo['signo'] !== SIGUSR1) { + var_dump($signinfo); + } + $this->assertEquals(SIGUSR1, $signinfo['signo']); + $this->assertArrayHasKey('errno', $signinfo); + $this->assertIsInt($signinfo['errno']); + $this->assertArrayHasKey('code', $signinfo); + $this->assertIsInt($signinfo['code']); + $this->assertArrayHasKey('pid', $signinfo); + $this->assertIsInt($signinfo['pid']); + $this->assertArrayHasKey('uid', $signinfo); + $this->assertIsInt($signinfo['uid']); } $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); - $this->assertEquals($this->manager->getEventDispatcher(), $dispatcher); + $this->assertEquals($this->processManager->getEventDispatcher(), $dispatcher); } ); - $this->manager->fork(function (SharedMemory $sharedMem) { + $this->processManager->fork(function (SharedMemory $sharedMem) { $sharedMem->signal(SIGUSR1); }); - $this->manager->wait(); + $this->processManager->wait(); $this->assertTrue($signaled); } + + public function testManyListenersOneSignal(): void + { + $sigFirst = false; + $sigSecond = false; + + $this->processManager->addListener(SIGUSR1, function () use (&$sigFirst) { + $sigFirst = true; + }); + + $this->processManager->addListener(SIGUSR1, function () use (&$sigSecond) { + $sigSecond = true; + }); + + $this->processManager->fork(function (SharedMemory $sharedMem) { + $sharedMem->signal(SIGUSR1); + }); + + $this->processManager->wait(); + + $this->assertTrue($sigFirst); + $this->assertTrue($sigSecond); + } + + public function testPreviousSignalHandler(): void + { + $testSig = SIGUSR1; + $sigOrig = 0; + $sigNew = 0; + + $origSigHandler = function () use (&$sigOrig) { + ++$sigOrig; + }; + + $newSigHandler = function () use (&$sigNew) { + ++$sigNew; + }; + + pcntl_signal($testSig, $origSigHandler); + + $this->assertEquals($origSigHandler, pcntl_signal_get_handler($testSig)); + $this->assertEquals(0, $sigOrig); + $this->assertEquals(0, $sigNew); + + posix_kill(posix_getpid(), $testSig); + + $this->assertEquals(1, $sigOrig); + $this->assertEquals(0, $sigNew); + + $this->processManager->addListener($testSig, $newSigHandler); + + $currSigHandler = pcntl_signal_get_handler($testSig); + $this->assertNotEquals($origSigHandler, $currSigHandler); + $this->assertEquals(1, $sigOrig); + $this->assertEquals(0, $sigNew); + + $this->processManager->fork(function (SharedMemory $sharedMem) use (&$testSig) { + $sharedMem->signal($testSig); + }); + + $this->processManager->wait(); + + $this->assertEquals(2, $sigOrig); + $this->assertEquals(1, $sigNew); + + posix_kill(posix_getpid(), $testSig); + + $this->assertEquals(3, $sigOrig); + $this->assertEquals(2, $sigNew); + + $this->processManager->getEventDispatcher()->removeSignalListener($testSig, $newSigHandler); + + $currSigHandler = pcntl_signal_get_handler($testSig); + $this->assertNotEquals($origSigHandler, $currSigHandler); + $this->assertEquals(3, $sigOrig); + $this->assertEquals(2, $sigNew); + + posix_kill(posix_getpid(), $testSig); + + $this->assertEquals(4, $sigOrig); + $this->assertEquals(2, $sigNew); + + $this->processManager->addListener($testSig, $newSigHandler); + + $currSigHandler = pcntl_signal_get_handler($testSig); + $this->assertNotEquals($origSigHandler, $currSigHandler); + $this->assertEquals(4, $sigOrig); + $this->assertEquals(2, $sigNew); + + posix_kill(posix_getpid(), $testSig); + + $this->assertEquals(5, $sigOrig); + $this->assertEquals(3, $sigNew); + + $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); + + $this->assertEquals($origSigHandler, pcntl_signal_get_handler($testSig)); + $this->assertEquals(5, $sigOrig); + $this->assertEquals(3, $sigNew); + + posix_kill(posix_getpid(), $testSig); + + $this->assertEquals(6, $sigOrig); + $this->assertEquals(3, $sigNew); + } } From d18f9c6c64e1cbcaea8cbc9e586209ab9a2933eb Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Wed, 1 Jan 2020 19:41:07 +0100 Subject: [PATCH 04/16] #5: Rename dispatcher classes --- ...Dispatcher.php => SignalEventDispatcher.php} | 7 ++++--- ...e.php => SignalEventDispatcherInterface.php} | 6 +++--- ...Trait.php => SignalEventDispatcherTrait.php} | 17 ++++++++++++++++- .../EventDispatcher/WrappedEventDispatcher.php | 8 ++++---- src/Spork/ProcessManager.php | 13 ++++++++----- tests/Spork/SignalTest.php | 7 ++----- 6 files changed, 37 insertions(+), 21 deletions(-) rename src/Spork/EventDispatcher/{EventDispatcher.php => SignalEventDispatcher.php} (70%) rename src/Spork/EventDispatcher/{EventDispatcherInterface.php => SignalEventDispatcherInterface.php} (90%) rename src/Spork/EventDispatcher/{EventDispatcherTrait.php => SignalEventDispatcherTrait.php} (94%) diff --git a/src/Spork/EventDispatcher/EventDispatcher.php b/src/Spork/EventDispatcher/SignalEventDispatcher.php similarity index 70% rename from src/Spork/EventDispatcher/EventDispatcher.php rename to src/Spork/EventDispatcher/SignalEventDispatcher.php index 500b518..8c122a3 100644 --- a/src/Spork/EventDispatcher/EventDispatcher.php +++ b/src/Spork/EventDispatcher/SignalEventDispatcher.php @@ -13,13 +13,14 @@ namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcher as BaseClass; +use Symfony\Component\EventDispatcher\EventDispatcher; /** * Extends the core event dispatcher with signal handling capabilities. Add and * remove signal listeners or dispatch a signal directly. */ -class EventDispatcher extends BaseClass implements EventDispatcherInterface +class SignalEventDispatcher extends EventDispatcher implements + SignalEventDispatcherInterface { - use EventDispatcherTrait; + use SignalEventDispatcherTrait; } diff --git a/src/Spork/EventDispatcher/EventDispatcherInterface.php b/src/Spork/EventDispatcher/SignalEventDispatcherInterface.php similarity index 90% rename from src/Spork/EventDispatcher/EventDispatcherInterface.php rename to src/Spork/EventDispatcher/SignalEventDispatcherInterface.php index 87e2242..644aff7 100644 --- a/src/Spork/EventDispatcher/EventDispatcherInterface.php +++ b/src/Spork/EventDispatcher/SignalEventDispatcherInterface.php @@ -13,16 +13,16 @@ namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; /** * Extends the core event dispatcher interface with signal handling * capabilities. Add and remove signal listeners or dispatch a signal directly. */ -interface EventDispatcherInterface extends BaseInterface +interface SignalEventDispatcherInterface extends EventDispatcherInterface { /** - * The signal handler. + * Signal handler that dispatches events. * * @param int $signo The signal being handled. * @param mixed $signinfo If operating systems supports siginfo_t diff --git a/src/Spork/EventDispatcher/EventDispatcherTrait.php b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php similarity index 94% rename from src/Spork/EventDispatcher/EventDispatcherTrait.php rename to src/Spork/EventDispatcher/SignalEventDispatcherTrait.php index 8427e56..fe101cd 100644 --- a/src/Spork/EventDispatcher/EventDispatcherTrait.php +++ b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php @@ -20,7 +20,7 @@ * Partial common implementation of the signal related EventDispatcherInterface * interface. */ -trait EventDispatcherTrait +trait SignalEventDispatcherTrait { /** * Holds signal handler wrappers to preserve a potentially already existing @@ -30,6 +30,16 @@ trait EventDispatcherTrait */ private $sigHandlerWrappers = []; + /** + * Remove all installed signal handler wrappers. + * + * @return void + */ + public function __destruct() + { + $this->removeSignalHandlerWrappers(); + } + /** * {@inheritDoc} */ @@ -67,6 +77,11 @@ public function removeSignalListener(int $signo, callable $listener): void $this->removeListener(SignalEvent::getEventName($signo), $listener); } + /** + * Removes all signal handlers this class has attached. + * + * @return void + */ public function removeSignalHandlerWrappers(): void { foreach ($this->sigHandlerWrappers as $signo => $sigHandlerWrapper) { diff --git a/src/Spork/EventDispatcher/WrappedEventDispatcher.php b/src/Spork/EventDispatcher/WrappedEventDispatcher.php index b54f44f..5312f88 100644 --- a/src/Spork/EventDispatcher/WrappedEventDispatcher.php +++ b/src/Spork/EventDispatcher/WrappedEventDispatcher.php @@ -13,15 +13,15 @@ namespace Spork\EventDispatcher; -use Symfony\Component\EventDispatcher\EventDispatcherInterface as BaseInterface; +use Symfony\Component\EventDispatcher\EventDispatcherInterface; use Symfony\Component\EventDispatcher\EventSubscriberInterface; /** * Wraps another event dispatcher, adding signal handling capabilities to it. */ -class WrappedEventDispatcher implements EventDispatcherInterface +class WrappedEventDispatcher implements SignalEventDispatcherInterface { - use EventDispatcherTrait; + use SignalEventDispatcherTrait; /** * The wrapped event dispatcher. @@ -36,7 +36,7 @@ class WrappedEventDispatcher implements EventDispatcherInterface * @param \Symfony\Component\EventDispatcher\EventDispatcherInterface $delegate * The wrapped event dispatcher. */ - public function __construct(BaseInterface $delegate) + public function __construct(EventDispatcherInterface $delegate) { $this->delegate = $delegate; } diff --git a/src/Spork/ProcessManager.php b/src/Spork/ProcessManager.php index 0355874..1384630 100644 --- a/src/Spork/ProcessManager.php +++ b/src/Spork/ProcessManager.php @@ -13,9 +13,9 @@ use InvalidArgumentException; use Spork\Batch\Strategy\StrategyInterface; -use Spork\EventDispatcher\EventDispatcher; -use Spork\EventDispatcher\EventDispatcherInterface; use Spork\EventDispatcher\Events; +use Spork\EventDispatcher\SignalEventDispatcher; +use Spork\EventDispatcher\SignalEventDispatcherInterface; use Spork\Exception\ProcessControlException; use Spork\Exception\UnexpectedTypeException; use Spork\Util\Error; @@ -34,9 +34,12 @@ class ProcessManager /** @var Fork[] */ private $forks; - public function __construct(EventDispatcherInterface $dispatcher = null, Factory $factory = null, $debug = false) - { - $this->dispatcher = $dispatcher ?: new EventDispatcher(); + public function __construct( + SignalEventDispatcherInterface $dispatcher = null, + Factory $factory = null, + $debug = false + ) { + $this->dispatcher = $dispatcher ?: new SignalEventDispatcher(); $this->factory = $factory ?: new Factory(); $this->debug = $debug; $this->zombieOkay = false; diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index 13dd5a2..aaeb47a 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -14,8 +14,8 @@ namespace Spork; use PHPUnit\Framework\TestCase; -use Spork\EventDispatcher\EventDispatcher; use Spork\EventDispatcher\SignalEvent; +use Spork\EventDispatcher\SignalEventDispatcherInterface; class SignalTest extends TestCase { @@ -45,14 +45,11 @@ protected function setUp(): void $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); $this->async = pcntl_async_signals(); pcntl_async_signals(true); - $this->processManager = new ProcessManager(); } protected function tearDown(): void { - $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); - pcntl_async_signals($this->async); $this->errorReporting = error_reporting($this->errorReporting); } @@ -66,7 +63,7 @@ public function testSingleListenerOneSignal() function ( SignalEvent $event, string $eventName, - EventDispatcher $dispatcher + SignalEventDispatcherInterface $dispatcher ) use (&$signaled) { $signaled = true; From 14fb56d6a1193a118d331466af997247a1e0e2d5 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Wed, 1 Jan 2020 22:40:34 +0100 Subject: [PATCH 05/16] #5: Improve test coverage of signal handlers --- .../SignalEventDispatcherTrait.php | 4 +-- tests/Spork/SignalTest.php | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php index fe101cd..a338e7f 100644 --- a/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php +++ b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php @@ -150,7 +150,7 @@ private function getSignalHandler(int $signo) $signo, $error, $strerror - )); + ), $error); } return $signalHandler; @@ -186,7 +186,7 @@ private function setSignalHandler(int $signo, $handler): void $signo, $error, $strerror - )); + ), $error); } } } diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index aaeb47a..9a89073 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -14,8 +14,11 @@ namespace Spork; use PHPUnit\Framework\TestCase; +use ReflectionClass; +use ReflectionObject; use Spork\EventDispatcher\SignalEvent; use Spork\EventDispatcher\SignalEventDispatcherInterface; +use UnexpectedValueException; class SignalTest extends TestCase { @@ -204,4 +207,34 @@ public function testPreviousSignalHandler(): void $this->assertEquals(6, $sigOrig); $this->assertEquals(3, $sigNew); } + + public function testSignalHandlerInstallFailure(): void + { + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessageRegExp( + '/Could not get installed signal handler for signal 255./' + ); + $this->expectExceptionCode(22); + + $this->processManager->addListener(255, function () { + // Do nothing. + }); + } + + public function testSignalHandlerInstallErrorHandling(): void + { + $this->expectException(UnexpectedValueException::class); + $this->expectExceptionMessageRegExp( + '/Could not install signal handler for signal 255./' + ); + $this->expectExceptionCode(22); + + $reflection = new ReflectionObject($this->processManager->getEventDispatcher()); + $method = $reflection->getMethod('setSignalHandler'); + $method->setAccessible(true); + + $method->invokeArgs($this->processManager->getEventDispatcher(), [255, function () { + // Do nothing. + }]); + } } From 55df25c1266271c3a563179e347868ef333e6df9 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sat, 11 Jan 2020 11:24:45 +0100 Subject: [PATCH 06/16] #5: Added coverage report to gitignore --- .gitattributes | 1 + .gitignore | 1 + composer.json | 1 + 3 files changed, 3 insertions(+) diff --git a/.gitattributes b/.gitattributes index 4c195c4..6cafeb8 100644 --- a/.gitattributes +++ b/.gitattributes @@ -2,6 +2,7 @@ /.github export-ignore /.gitignore export-ignore /.travis.yml export-ignore +/coverage export-ignore /php_cs.dist export-ignore /phpstan.neon.dist export-ignore /phpunit.xml.dist export-ignore diff --git a/.gitignore b/.gitignore index 0965958..e0b6acd 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ /.php_cs /.phpunit.result.cache /composer.lock +/coverage /phpstan.neon /phpunit.xml /vendor diff --git a/composer.json b/composer.json index 45773d7..745cedb 100644 --- a/composer.json +++ b/composer.json @@ -69,6 +69,7 @@ "/.github", "/.gitignore", "/.travis.yml", + "/coverage", "/php_cs.dist", "/phpstan.neon.dist", "/phpunit.xml.dist", From 74f783e3b8be97dd0856ab21965ed4107a831a60 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sat, 11 Jan 2020 11:54:44 +0100 Subject: [PATCH 07/16] #5: Add SignalHandlerWrapper tests --- .../WrappedEventDispatcher.php | 2 +- .../Spork/Signal/SignalHandlerWrapperTest.php | 61 +++++++++++++++++++ tests/Spork/SignalTest.php | 1 - 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 tests/Spork/Signal/SignalHandlerWrapperTest.php diff --git a/src/Spork/EventDispatcher/WrappedEventDispatcher.php b/src/Spork/EventDispatcher/WrappedEventDispatcher.php index 5312f88..b563d02 100644 --- a/src/Spork/EventDispatcher/WrappedEventDispatcher.php +++ b/src/Spork/EventDispatcher/WrappedEventDispatcher.php @@ -44,7 +44,7 @@ public function __construct(EventDispatcherInterface $delegate) /** * {@inheritDoc} */ - public function dispatch($event, string $eventName = null) + public function dispatch($event, string $eventName = null): object { return call_user_func([$this->delegate, 'dispatch'], $event, $eventName); } diff --git a/tests/Spork/Signal/SignalHandlerWrapperTest.php b/tests/Spork/Signal/SignalHandlerWrapperTest.php new file mode 100644 index 0000000..ac1d671 --- /dev/null +++ b/tests/Spork/Signal/SignalHandlerWrapperTest.php @@ -0,0 +1,61 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\Signal; + +use PHPUnit\Framework\TestCase; + +class SignalHandlerWrapperTest extends TestCase +{ + public function testSignalHandlerWrapper() + { + $prevCalls = 0; + $currCalls = 0; + + $previous = function () use (&$prevCalls) { + ++$prevCalls; + }; + + $current = function () use (&$currCalls) { + ++$currCalls; + }; + + $sigHandlerWrapper = new SignalHandlerWrapper($previous, $current); + $prev = $sigHandlerWrapper->getPrevious(); + $curr = $sigHandlerWrapper->getCurrent(); + + $this->assertEquals(0, $prevCalls); + $this->assertEquals(0, $currCalls); + $this->assertSame($previous, $prev); + $this->assertSame($current, $curr); + + $prev(); + $curr(); + + $this->assertEquals(1, $prevCalls); + $this->assertEquals(1, $currCalls); + + $wrapper = $sigHandlerWrapper->getWrapper(); + $this->assertIsCallable($wrapper); + + $wrapper(SIGUSR1, null); + + $this->assertEquals(2, $prevCalls); + $this->assertEquals(2, $currCalls); + + $sigHandlerWrapper(SIGUSR1, null); + + $this->assertEquals(3, $prevCalls); + $this->assertEquals(3, $currCalls); + } +} diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index 9a89073..8d7359c 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -14,7 +14,6 @@ namespace Spork; use PHPUnit\Framework\TestCase; -use ReflectionClass; use ReflectionObject; use Spork\EventDispatcher\SignalEvent; use Spork\EventDispatcher\SignalEventDispatcherInterface; From 2f18c2c928d3f672bba31c5531edd07070914661 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sat, 11 Jan 2020 12:57:03 +0100 Subject: [PATCH 08/16] #5: Improve static analyzer and min version --- composer.json | 8 ++++---- phpstan.neon.dist | 2 +- src/Spork/ProcessManager.php | 2 +- src/Spork/Util/Error.php | 12 ++++++++---- tests/Spork/ProcessManagerTest.php | 3 ++- tests/Spork/Signal/SignalHandlerWrapperTest.php | 2 ++ 6 files changed, 18 insertions(+), 11 deletions(-) diff --git a/composer.json b/composer.json index 745cedb..1d643e1 100644 --- a/composer.json +++ b/composer.json @@ -36,17 +36,17 @@ }, "require": { "php": ">=7.2.0", - "symfony/event-dispatcher": ">=3.0.0" + "symfony/event-dispatcher": "^4.0.0 || ^5.0.0" }, "require-dev": { "ext-pcntl": "*", "ext-posix": "*", "ext-shmop": "*", "friendsofphp/php-cs-fixer": "^2.16", - "phpstan/phpstan": "^0.11.19", - "phpunit/phpunit": "^8.4", + "phpstan/phpstan": "^0.12.4", + "phpunit/phpunit": "^8.5", "squizlabs/php_codesniffer": "^3.5", - "symfony/var-dumper": "^4.0.0" + "symfony/var-dumper": "^5.0" }, "autoload": { "psr-4": { diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 986dc22..514c28f 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -1,5 +1,5 @@ parameters: - level: max + level: 5 paths: - src - tests diff --git a/src/Spork/ProcessManager.php b/src/Spork/ProcessManager.php index 1384630..cf27328 100644 --- a/src/Spork/ProcessManager.php +++ b/src/Spork/ProcessManager.php @@ -112,7 +112,7 @@ public function fork($callable) $message = new ExitMessage(); // phone home on shutdown - register_shutdown_function(function () use ($shm, $message) { + register_shutdown_function(function () use ($shm, $message): void { $status = null; try { diff --git a/src/Spork/Util/Error.php b/src/Spork/Util/Error.php index b4286d7..cab5df8 100644 --- a/src/Spork/Util/Error.php +++ b/src/Spork/Util/Error.php @@ -1,17 +1,21 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork\Util; -class Error implements \Serializable +use Serializable; + +class Error implements Serializable { private $class; private $message; @@ -21,7 +25,7 @@ class Error implements \Serializable public static function fromException(\Exception $e) { - $flat = new static(); + $flat = new self(); $flat->setClass(get_class($e)); $flat->setMessage($e->getMessage()); $flat->setFile($e->getFile()); diff --git a/tests/Spork/ProcessManagerTest.php b/tests/Spork/ProcessManagerTest.php index 03c9fc7..784c6a7 100644 --- a/tests/Spork/ProcessManagerTest.php +++ b/tests/Spork/ProcessManagerTest.php @@ -13,6 +13,7 @@ use Exception; use PHPUnit\Framework\TestCase; +use stdClass; class ProcessManagerTest extends TestCase { @@ -74,7 +75,7 @@ public function testFailCallbacks() public function testObjectReturn() { - $mock = $this->getMockBuilder('Unserializable')->setMethods(['__sleep'])->getMock(); + $mock = $this->getMockBuilder(stdClass::class)->setMethods(['__sleep'])->getMock(); $mock->method('__sleep')->willThrowException(new Exception("Hey, don\'t serialize me!")); $fork = $this->manager->fork(function () use (&$mock) { diff --git a/tests/Spork/Signal/SignalHandlerWrapperTest.php b/tests/Spork/Signal/SignalHandlerWrapperTest.php index ac1d671..3e8b060 100644 --- a/tests/Spork/Signal/SignalHandlerWrapperTest.php +++ b/tests/Spork/Signal/SignalHandlerWrapperTest.php @@ -31,7 +31,9 @@ public function testSignalHandlerWrapper() }; $sigHandlerWrapper = new SignalHandlerWrapper($previous, $current); + /** @var callable $prev */ $prev = $sigHandlerWrapper->getPrevious(); + /** @var callable $curr */ $curr = $sigHandlerWrapper->getCurrent(); $this->assertEquals(0, $prevCalls); From b53bef3848e840052a6210959dc18ad7f7a20d37 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Wed, 22 Jan 2020 20:20:44 +0100 Subject: [PATCH 09/16] #5: Add phan and improve test coverage --- .phan/config.php | 395 ++++++++++++++++++ composer.json | 10 +- .../SignalEventDispatcherTrait.php | 6 +- .../WrappedEventDispatcherTest.php | 110 +++++ tests/Spork/SignalTest.php | 10 +- 5 files changed, 525 insertions(+), 6 deletions(-) create mode 100644 .phan/config.php create mode 100644 tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php diff --git a/.phan/config.php b/.phan/config.php new file mode 100644 index 0000000..d6e1ccc --- /dev/null +++ b/.phan/config.php @@ -0,0 +1,395 @@ + '7.4', + + // If enabled, missing properties will be created when + // they are first seen. If false, we'll report an + // error message if there is an attempt to write + // to a class property that wasn't explicitly + // defined. + 'allow_missing_properties' => false, + + // If enabled, null can be cast to any type and any + // type can be cast to null. Setting this to true + // will cut down on false positives. + 'null_casts_as_any_type' => false, + + // If enabled, allow null to be cast as any array-like type. + // + // This is an incremental step in migrating away from `null_casts_as_any_type`. + // If `null_casts_as_any_type` is true, this has no effect. + 'null_casts_as_array' => false, + + // If enabled, allow any array-like type to be cast to null. + // This is an incremental step in migrating away from `null_casts_as_any_type`. + // If `null_casts_as_any_type` is true, this has no effect. + 'array_casts_as_null' => false, + + // If enabled, scalars (int, float, bool, string, null) + // are treated as if they can cast to each other. + // This does not affect checks of array keys. See `scalar_array_key_cast`. + 'scalar_implicit_cast' => false, + + // If enabled, any scalar array keys (int, string) + // are treated as if they can cast to each other. + // E.g. `array` can cast to `array` and vice versa. + // Normally, a scalar type such as int could only cast to/from int and mixed. + 'scalar_array_key_cast' => false, + + // If this has entries, scalars (int, float, bool, string, null) + // are allowed to perform the casts listed. + // + // E.g. `['int' => ['float', 'string'], 'float' => ['int'], 'string' => ['int'], 'null' => ['string']]` + // allows casting null to a string, but not vice versa. + // (subset of `scalar_implicit_cast`) + 'scalar_implicit_partial' => [], + + // If enabled, Phan will warn if **any** type in a method invocation's object + // is definitely not an object, + // or if **any** type in an invoked expression is not a callable. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_method_checking' => true, + + // If enabled, Phan will warn if **any** type of the object expression for a property access + // does not contain that property. + 'strict_object_checking' => true, + + // If enabled, Phan will warn if **any** type in the argument's union type + // cannot be cast to a type in the parameter's expected union type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_param_checking' => true, + + // If enabled, Phan will warn if **any** type in a property assignment's union type + // cannot be cast to a type in the property's declared union type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_property_checking' => true, + + // If enabled, Phan will warn if **any** type in a returned value's union type + // cannot be cast to the declared return type. + // Setting this to true will introduce numerous false positives + // (and reveal some bugs). + 'strict_return_checking' => true, + + // If true, seemingly undeclared variables in the global + // scope will be ignored. + // + // This is useful for projects with complicated cross-file + // globals that you have no hope of fixing. + 'ignore_undeclared_variables_in_global_scope' => false, + + // Set this to false to emit `PhanUndeclaredFunction` issues for internal functions that Phan has signatures for, + // but aren't available in the codebase, or from Reflection. + // (may lead to false positives if an extension isn't loaded) + // + // If this is true(default), then Phan will not warn. + // + // Even when this is false, Phan will still infer return values and check parameters of internal functions + // if Phan has the signatures. + 'ignore_undeclared_functions_with_known_signatures' => false, + + // Backwards Compatibility Checking. This is slow + // and expensive, but you should consider running + // it before upgrading your version of PHP to a + // new version that has backward compatibility + // breaks. + // + // If you are migrating from PHP 5 to PHP 7, + // you should also look into using + // [php7cc (no longer maintained)](https://github.com/sstalle/php7cc) + // and [php7mar](https://github.com/Alexia/php7mar), + // which have different backwards compatibility checks. + 'backward_compatibility_checks' => false, + + // If true, check to make sure the return type declared + // in the doc-block (if any) matches the return type + // declared in the method signature. + 'check_docblock_signature_return_type_match' => true, + + // If true, make narrowed types from phpdoc params override + // the real types from the signature, when real types exist. + // (E.g. allows specifying desired lists of subclasses, + // or to indicate a preference for non-nullable types over nullable types) + // + // Affects analysis of the body of the method and the param types passed in by callers. + // + // (*Requires `check_docblock_signature_param_type_match` to be true*) + 'prefer_narrowed_phpdoc_param_type' => true, + + // (*Requires `check_docblock_signature_return_type_match` to be true*) + // + // If true, make narrowed types from phpdoc returns override + // the real types from the signature, when real types exist. + // + // (E.g. allows specifying desired lists of subclasses, + // or to indicate a preference for non-nullable types over nullable types) + // + // This setting affects the analysis of return statements in the body of the + // method and the return types passed in by callers. + 'prefer_narrowed_phpdoc_return_type' => true, + + // If enabled, check all methods that override a + // parent method to make sure its signature is + // compatible with the parent's. + // + // This check can add quite a bit of time to the analysis. + // + // This will also check if final methods are overridden, etc. + 'analyze_signature_compatibility' => true, + + // This setting maps case-insensitive strings to union types. + // + // This is useful if a project uses phpdoc that differs from the phpdoc2 standard. + // + // If the corresponding value is the empty string, + // then Phan will ignore that union type (E.g. can ignore 'the' in `@return the value`) + // + // If the corresponding value is not empty, + // then Phan will act as though it saw the corresponding UnionTypes(s) + // when the keys show up in a UnionType of `@param`, `@return`, `@var`, `@property`, etc. + // + // This matches the **entire string**, not parts of the string. + // (E.g. `@return the|null` will still look for a class with the name `the`, + // but `@return the` will be ignored with the below setting) + // + // (These are not aliases, this setting is ignored outside of doc comments). + // (Phan does not check if classes with these names exist) + // + // Example setting: `['unknown' => '', 'number' => 'int|float', 'char' => 'string', 'long' => 'int', 'the' => '']` + 'phpdoc_type_mapping' => [], + + // Set to true in order to attempt to detect dead + // (unreferenced) code. Keep in mind that the + // results will only be a guess given that classes, + // properties, constants and methods can be referenced + // as variables (like `$class->$property` or + // `$class->$method()`) in ways that we're unable + // to make sense of. + 'dead_code_detection' => true, + + // Set to true in order to attempt to detect unused variables. + // `dead_code_detection` will also enable unused variable detection. + // + // This has a few known false positives, e.g. for loops or branches. + 'unused_variable_detection' => true, + + // Set to true in order to attempt to detect redundant and impossible conditions. + // + // This has some false positives involving loops, + // variables set in branches of loops, and global variables. + 'redundant_condition_detection' => true, + + // If enabled, Phan will act as though it's certain of real return types of + // a subset of internal functions, even if those return types aren't + // available in reflection (real types were taken from php 7.3 or 8.0-dev, + // depending on target_php_version). + // + // Note that with php 7 and earlier, php would return null or false for many + // internal functions if the argument types or counts were incorrect. + // As a result, enabling this setting with target_php_version 8.0 may result + // in false positives for `--redundant-condition-detection` when codebases + // also support php 7.x. + 'assume_real_types_for_internal_functions' => true, + + // If true, this runs a quick version of checks that takes less + // time at the cost of not running as thorough + // of an analysis. You should consider setting this + // to true only when you wish you had more **undiagnosed** issues + // to fix in your code base. + // + // In quick-mode the scanner doesn't rescan a function + // or a method's code block every time a call is seen. + // This means that the problem here won't be detected: + // + // ```php + // false, + + // Enable or disable support for generic templated + // class types. + 'generic_types_enabled' => true, + + // Override to hardcode existence and types of (non-builtin) globals in the global scope. + // Class names should be prefixed with `\`. + // + // (E.g. `['_FOO' => '\FooClass', 'page' => '\PageClass', 'userId' => 'int']`) + 'globals_type_map' => [], + + // The minimum severity level to report on. This can be + // set to `Issue::SEVERITY_LOW`, `Issue::SEVERITY_NORMAL` or + // `Issue::SEVERITY_CRITICAL`. Setting it to only + // critical issues is a good place to start on a big + // sloppy mature code base. + 'minimum_severity' => Issue::SEVERITY_LOW, + + // Add any issue types (such as `'PhanUndeclaredMethod'`) + // to this black-list to inhibit them from being reported. + 'suppress_issue_types' => [], + + // A regular expression to match files to be excluded + // from parsing and analysis and will not be read at all. + // + // This is useful for excluding groups of test or example + // directories/files, unanalyzable files, or files that + // can't be removed for whatever reason. + // (e.g. `'@Test\.php$@'`, or `'@vendor/.*/(tests|Tests)/@'`) + 'exclude_file_regex' => '@^vendor/.*/(tests?|Tests?)/@', + + // A list of files that will be excluded from parsing and analysis + // and will not be read at all. + // + // This is useful for excluding hopelessly unanalyzable + // files that can't be removed for whatever reason. + 'exclude_file_list' => [], + + // A directory list that defines files that will be excluded + // from static analysis, but whose class and method + // information should be included. + // + // Generally, you'll want to include the directories for + // third-party code (such as "vendor/") in this list. + // + // n.b.: If you'd like to parse but not analyze 3rd + // party code, directories containing that code + // should be added to the `directory_list` as well as + // to `exclude_analysis_directory_list`. + 'exclude_analysis_directory_list' => [ + 'vendor/', + ], + + // Enable this to enable checks of require/include statements referring to valid paths. + 'enable_include_path_checks' => true, + + // The number of processes to fork off during the analysis + // phase. + 'processes' => 1, + + // List of case-insensitive file extensions supported by Phan. + // (e.g. `['php', 'html', 'htm']`) + 'analyzed_file_extensions' => [ + 'php', + ], + + // You can put paths to stubs of internal extensions in this config option. + // If the corresponding extension is **not** loaded, then Phan will use the stubs instead. + // Phan will continue using its detailed type annotations, + // but load the constants, classes, functions, and classes (and their Reflection types) + // from these stub files (doubling as valid php files). + // Use a different extension from php to avoid accidentally loading these. + // The `tools/make_stubs` script can be used to generate your own stubs (compatible with php 7.0+ right now) + // + // (e.g. `['xdebug' => '.phan/internal_stubs/xdebug.phan_php']`) + 'autoload_internal_extension_signatures' => [], + + // A list of plugin files to execute. + // + // Plugins which are bundled with Phan can be added here by providing their + // name (e.g. `'AlwaysReturnPlugin'`) + // + // Documentation about available bundled plugins can be found + // [here](https://github.com/phan/phan/tree/master/.phan/plugins). + // + // Alternately, you can pass in the full path to a PHP file with the + // plugin's implementation (e.g. + // `'vendor/phan/phan/.phan/plugins/AlwaysReturnPlugin.php'`) + 'plugins' => [ + 'AlwaysReturnPlugin', + 'DollarDollarPlugin', + 'DuplicateArrayKeyPlugin', + 'DuplicateExpressionPlugin', + 'PregRegexCheckerPlugin', + 'PrintfCheckerPlugin', + 'SleepCheckerPlugin', + 'UnreachableCodePlugin', + 'UseReturnValuePlugin', + 'EmptyStatementListPlugin', + 'StrictComparisonPlugin', + 'LoopVariableReusePlugin', + ], + + // A list of directories that should be parsed for class and + // method information. After excluding the directories + // defined in `exclude_analysis_directory_list`, the remaining + // files will be statically analyzed for errors. + // + // Thus, both first-party and third-party code being used by + // your application should be included in this list. + 'directory_list' => [ + 'src/Spork', + 'vendor/symfony/event-dispatcher', + 'vendor/symfony/var-dumper', + ], + + // A list of individual files to include in analysis + // with a path relative to the root directory of the + // project. + 'file_list' => [], +]; diff --git a/composer.json b/composer.json index 1d643e1..5de3884 100644 --- a/composer.json +++ b/composer.json @@ -35,7 +35,7 @@ "rss": "https://github.com/TheLevti/spork/commits/master.atom" }, "require": { - "php": ">=7.2.0", + "php": "^7.2.0", "symfony/event-dispatcher": "^4.0.0 || ^5.0.0" }, "require-dev": { @@ -43,6 +43,7 @@ "ext-posix": "*", "ext-shmop": "*", "friendsofphp/php-cs-fixer": "^2.16", + "phan/phan": "^2.4", "phpstan/phpstan": "^0.12.4", "phpunit/phpunit": "^8.5", "squizlabs/php_codesniffer": "^3.5", @@ -78,5 +79,12 @@ }, "config": { "sort-packages": true + }, + "scripts": { + "cs": "vendor/bin/phpcs --standard=PSR12 src/ tests/", + "csf": "vendor/bin/php-cs-fixer fix", + "static": "vendor/bin/phpstan analyse", + "test": "vendor/bin/phpunit", + "coverage": "vendor/bin/phpunit --coverage-html coverage" } } diff --git a/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php index a338e7f..ae6639e 100644 --- a/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php +++ b/src/Spork/EventDispatcher/SignalEventDispatcherTrait.php @@ -26,7 +26,7 @@ trait SignalEventDispatcherTrait * Holds signal handler wrappers to preserve a potentially already existing * signal handler. * - * @var array<\Spork\Signal\SignalHandlerWrapper> $sigHandlerWrappers + * @var array $sigHandlerWrappers */ private $sigHandlerWrappers = []; @@ -149,7 +149,7 @@ private function getSignalHandler(int $signo) 'Could not get installed signal handler for signal %d. %d: %s', $signo, $error, - $strerror + (string)$strerror ), $error); } @@ -185,7 +185,7 @@ private function setSignalHandler(int $signo, $handler): void 'Could not install signal handler for signal %d. %d: %s', $signo, $error, - $strerror + (string)$strerror ), $error); } } diff --git a/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php b/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php new file mode 100644 index 0000000..465b5ca --- /dev/null +++ b/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php @@ -0,0 +1,110 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\EventDispatcher; + +use PHPUnit\Framework\TestCase; +use Spork\ProcessManager; +use Spork\SharedMemory; +use Symfony\Component\EventDispatcher\EventDispatcher; + +class WrappedEventDispatcherTest extends TestCase +{ + /** + * Process manager instance. + * + * @var \Spork\ProcessManager $processManager + */ + private $processManager; + + /** + * Holds the previous pcntl async signals value. + * + * @var bool $async + */ + private $async; + + /** + * Holds the previous error reporting configuration. + * + * @var int $errorReporting + */ + private $errorReporting; + + protected function setUp(): void + { + parent::setUp(); + + $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); + $this->async = pcntl_async_signals(); + pcntl_async_signals(true); + + $dispatcher = new EventDispatcher(); + $wrapped = new WrappedEventDispatcher($dispatcher); + $this->processManager = new ProcessManager($wrapped); + } + + protected function tearDown(): void + { + $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); + + pcntl_async_signals($this->async); + $this->errorReporting = error_reporting($this->errorReporting); + + parent::tearDown(); + } + + public function testSingleListenerOneSignal() + { + $signaled = false; + + $this->processManager->addListener( + SIGUSR1, + function ( + SignalEvent $event, + string $eventName + ) use (&$signaled) { + $signaled = true; + + $this->assertEquals(SIGUSR1, $event->getSigno()); + + $signinfo = $event->getSigninfo(); + if (is_array($signinfo)) { + $this->assertArrayHasKey('signo', $signinfo); + if ($signinfo['signo'] !== SIGUSR1) { + var_dump($signinfo); + } + $this->assertEquals(SIGUSR1, $signinfo['signo']); + $this->assertArrayHasKey('errno', $signinfo); + $this->assertIsInt($signinfo['errno']); + $this->assertArrayHasKey('code', $signinfo); + $this->assertIsInt($signinfo['code']); + $this->assertArrayHasKey('pid', $signinfo); + $this->assertIsInt($signinfo['pid']); + $this->assertArrayHasKey('uid', $signinfo); + $this->assertIsInt($signinfo['uid']); + } + + $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); + } + ); + + $this->processManager->fork(function (SharedMemory $sharedMem) { + $sharedMem->signal(SIGUSR1); + }); + + $this->processManager->wait(); + + $this->assertTrue($signaled); + } +} diff --git a/tests/Spork/SignalTest.php b/tests/Spork/SignalTest.php index 8d7359c..907177c 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/SignalTest.php @@ -16,7 +16,7 @@ use PHPUnit\Framework\TestCase; use ReflectionObject; use Spork\EventDispatcher\SignalEvent; -use Spork\EventDispatcher\SignalEventDispatcherInterface; +use Symfony\Component\EventDispatcher\EventDispatcher; use UnexpectedValueException; class SignalTest extends TestCase @@ -44,6 +44,8 @@ class SignalTest extends TestCase protected function setUp(): void { + parent::setUp(); + $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); $this->async = pcntl_async_signals(); pcntl_async_signals(true); @@ -52,8 +54,12 @@ protected function setUp(): void protected function tearDown(): void { + $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); + pcntl_async_signals($this->async); $this->errorReporting = error_reporting($this->errorReporting); + + parent::tearDown(); } public function testSingleListenerOneSignal() @@ -65,7 +71,7 @@ public function testSingleListenerOneSignal() function ( SignalEvent $event, string $eventName, - SignalEventDispatcherInterface $dispatcher + EventDispatcher $dispatcher ) use (&$signaled) { $signaled = true; From 6684b41069863a7b9cbf3600fc74534323d56079 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sun, 26 Jan 2020 10:43:46 +0100 Subject: [PATCH 10/16] #5: Improve test coverage of event dispatcher classes --- .../SignalEventDispatcherTest.php | 63 ++++++++++++++++++ .../SignalEventDispatcherTestTrait.php} | 53 ++------------- .../EventDispatcher/SignalEventSubscriber.php | 31 +++++++++ .../WrappedEventDispatcherTest.php | 64 ++++++++----------- 4 files changed, 126 insertions(+), 85 deletions(-) create mode 100644 tests/Spork/EventDispatcher/SignalEventDispatcherTest.php rename tests/Spork/{SignalTest.php => EventDispatcher/SignalEventDispatcherTestTrait.php} (84%) create mode 100644 tests/Spork/EventDispatcher/SignalEventSubscriber.php diff --git a/tests/Spork/EventDispatcher/SignalEventDispatcherTest.php b/tests/Spork/EventDispatcher/SignalEventDispatcherTest.php new file mode 100644 index 0000000..6f5d73e --- /dev/null +++ b/tests/Spork/EventDispatcher/SignalEventDispatcherTest.php @@ -0,0 +1,63 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\EventDispatcher; + +use PHPUnit\Framework\TestCase; +use Spork\ProcessManager; + +class SignalEventDispatcherTest extends TestCase +{ + use SignalEventDispatcherTestTrait; + + /** + * Process manager instance. + * + * @var \Spork\ProcessManager $processManager + */ + private $processManager; + + /** + * Holds the previous pcntl async signals value. + * + * @var bool $async + */ + private $async; + + /** + * Holds the previous error reporting configuration. + * + * @var int $errorReporting + */ + private $errorReporting; + + protected function setUp(): void + { + parent::setUp(); + + $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); + $this->async = pcntl_async_signals(); + pcntl_async_signals(true); + $this->processManager = new ProcessManager(); + } + + protected function tearDown(): void + { + $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); + + pcntl_async_signals($this->async); + $this->errorReporting = error_reporting($this->errorReporting); + + parent::tearDown(); + } +} diff --git a/tests/Spork/SignalTest.php b/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php similarity index 84% rename from tests/Spork/SignalTest.php rename to tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php index 907177c..ee1532b 100644 --- a/tests/Spork/SignalTest.php +++ b/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php @@ -11,57 +11,18 @@ declare(strict_types=1); -namespace Spork; +namespace Spork\EventDispatcher; -use PHPUnit\Framework\TestCase; use ReflectionObject; -use Spork\EventDispatcher\SignalEvent; +use Spork\SharedMemory; use Symfony\Component\EventDispatcher\EventDispatcher; use UnexpectedValueException; -class SignalTest extends TestCase +/** + * Common tests for signal event dispatchers. + */ +trait SignalEventDispatcherTestTrait { - /** - * Process manager instance. - * - * @var \Spork\ProcessManager $processManager - */ - private $processManager; - - /** - * Holds the previous pcntl async signals value. - * - * @var bool $async - */ - private $async; - - /** - * Holds the previous error reporting configuration. - * - * @var int $errorReporting - */ - private $errorReporting; - - protected function setUp(): void - { - parent::setUp(); - - $this->errorReporting = error_reporting(E_ALL & ~E_WARNING); - $this->async = pcntl_async_signals(); - pcntl_async_signals(true); - $this->processManager = new ProcessManager(); - } - - protected function tearDown(): void - { - $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); - - pcntl_async_signals($this->async); - $this->errorReporting = error_reporting($this->errorReporting); - - parent::tearDown(); - } - public function testSingleListenerOneSignal() { $signaled = false; @@ -95,7 +56,7 @@ function ( } $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); - $this->assertEquals($this->processManager->getEventDispatcher(), $dispatcher); + $this->assertTrue($dispatcher instanceof EventDispatcher); } ); diff --git a/tests/Spork/EventDispatcher/SignalEventSubscriber.php b/tests/Spork/EventDispatcher/SignalEventSubscriber.php new file mode 100644 index 0000000..c01299f --- /dev/null +++ b/tests/Spork/EventDispatcher/SignalEventSubscriber.php @@ -0,0 +1,31 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace Spork\EventDispatcher; + +use Symfony\Component\EventDispatcher\EventSubscriberInterface; + +class SignalEventSubscriber implements EventSubscriberInterface +{ + public static function getSubscribedEvents() + { + return [ + SignalEvent::getEventName(SIGUSR1) => ['onSigusr1', -128], + ]; + } + + public function onSigusr1() + { + // Do nothing. + } +} diff --git a/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php b/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php index 465b5ca..61223a3 100644 --- a/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php +++ b/tests/Spork/EventDispatcher/WrappedEventDispatcherTest.php @@ -15,11 +15,12 @@ use PHPUnit\Framework\TestCase; use Spork\ProcessManager; -use Spork\SharedMemory; use Symfony\Component\EventDispatcher\EventDispatcher; class WrappedEventDispatcherTest extends TestCase { + use SignalEventDispatcherTestTrait; + /** * Process manager instance. * @@ -64,47 +65,32 @@ protected function tearDown(): void parent::tearDown(); } - public function testSingleListenerOneSignal() + public function testDelegate() { - $signaled = false; - - $this->processManager->addListener( - SIGUSR1, - function ( - SignalEvent $event, - string $eventName - ) use (&$signaled) { - $signaled = true; - - $this->assertEquals(SIGUSR1, $event->getSigno()); - - $signinfo = $event->getSigninfo(); - if (is_array($signinfo)) { - $this->assertArrayHasKey('signo', $signinfo); - if ($signinfo['signo'] !== SIGUSR1) { - var_dump($signinfo); - } - $this->assertEquals(SIGUSR1, $signinfo['signo']); - $this->assertArrayHasKey('errno', $signinfo); - $this->assertIsInt($signinfo['errno']); - $this->assertArrayHasKey('code', $signinfo); - $this->assertIsInt($signinfo['code']); - $this->assertArrayHasKey('pid', $signinfo); - $this->assertIsInt($signinfo['pid']); - $this->assertArrayHasKey('uid', $signinfo); - $this->assertIsInt($signinfo['uid']); - } - - $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); - } - ); + $eventDispatcher = $this->processManager->getEventDispatcher(); + $sigEventSubscriber = new SignalEventSubscriber(); + $eventName = SignalEvent::getEventName(SIGUSR1); + + $this->assertFalse($eventDispatcher->hasListeners($eventName)); + + $eventDispatcher->addSubscriber($sigEventSubscriber); - $this->processManager->fork(function (SharedMemory $sharedMem) { - $sharedMem->signal(SIGUSR1); - }); + $this->assertTrue($eventDispatcher->hasListeners($eventName)); + + $eventListeners = $eventDispatcher->getListeners($eventName); + $this->assertIsArray($eventListeners); + $this->assertNotEmpty($eventListeners); + + $eventListener = $eventListeners[0]; + + $this->assertIsCallable($eventListeners[0]); + $this->assertEquals( + -128, + $eventDispatcher->getListenerPriority($eventName, $eventListener) + ); - $this->processManager->wait(); + $eventDispatcher->removeSubscriber($sigEventSubscriber); - $this->assertTrue($signaled); + $this->assertFalse($eventDispatcher->hasListeners($eventName)); } } From 8fb9f2fd0f9af472ccf4ba1ce7bd1009ac74d567 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sun, 26 Jan 2020 11:24:54 +0100 Subject: [PATCH 11/16] #5: Remove addEventListener method from process manager --- src/Spork/ProcessManager.php | 9 -- .../SignalEventDispatcherTestTrait.php | 85 +++++++++++++------ 2 files changed, 61 insertions(+), 33 deletions(-) diff --git a/src/Spork/ProcessManager.php b/src/Spork/ProcessManager.php index cf27328..24cb836 100644 --- a/src/Spork/ProcessManager.php +++ b/src/Spork/ProcessManager.php @@ -58,15 +58,6 @@ public function getEventDispatcher() return $this->dispatcher; } - public function addListener($eventName, $listener, $priority = 0) - { - if (is_integer($eventName)) { - $this->dispatcher->addSignalListener($eventName, $listener, $priority); - } else { - $this->dispatcher->addListener($eventName, $listener, $priority); - } - } - public function setDebug($debug) { $this->debug = $debug; diff --git a/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php b/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php index ee1532b..d0c4d80 100644 --- a/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php +++ b/tests/Spork/EventDispatcher/SignalEventDispatcherTestTrait.php @@ -27,7 +27,7 @@ public function testSingleListenerOneSignal() { $signaled = false; - $this->processManager->addListener( + $this->processManager->getEventDispatcher()->addSignalListener( SIGUSR1, function ( SignalEvent $event, @@ -55,7 +55,10 @@ function ( $this->assertIsInt($signinfo['uid']); } - $this->assertEquals(SignalEvent::getEventName(SIGUSR1), $eventName); + $this->assertEquals( + SignalEvent::getEventName(SIGUSR1), + $eventName + ); $this->assertTrue($dispatcher instanceof EventDispatcher); } ); @@ -74,13 +77,19 @@ public function testManyListenersOneSignal(): void $sigFirst = false; $sigSecond = false; - $this->processManager->addListener(SIGUSR1, function () use (&$sigFirst) { - $sigFirst = true; - }); + $this->processManager->getEventDispatcher()->addSignalListener( + SIGUSR1, + function () use (&$sigFirst) { + $sigFirst = true; + } + ); - $this->processManager->addListener(SIGUSR1, function () use (&$sigSecond) { - $sigSecond = true; - }); + $this->processManager->getEventDispatcher()->addSignalListener( + SIGUSR1, + function () use (&$sigSecond) { + $sigSecond = true; + } + ); $this->processManager->fork(function (SharedMemory $sharedMem) { $sharedMem->signal(SIGUSR1); @@ -108,7 +117,10 @@ public function testPreviousSignalHandler(): void pcntl_signal($testSig, $origSigHandler); - $this->assertEquals($origSigHandler, pcntl_signal_get_handler($testSig)); + $this->assertEquals( + $origSigHandler, + pcntl_signal_get_handler($testSig) + ); $this->assertEquals(0, $sigOrig); $this->assertEquals(0, $sigNew); @@ -117,16 +129,21 @@ public function testPreviousSignalHandler(): void $this->assertEquals(1, $sigOrig); $this->assertEquals(0, $sigNew); - $this->processManager->addListener($testSig, $newSigHandler); + $this->processManager->getEventDispatcher()->addSignalListener( + $testSig, + $newSigHandler + ); $currSigHandler = pcntl_signal_get_handler($testSig); $this->assertNotEquals($origSigHandler, $currSigHandler); $this->assertEquals(1, $sigOrig); $this->assertEquals(0, $sigNew); - $this->processManager->fork(function (SharedMemory $sharedMem) use (&$testSig) { - $sharedMem->signal($testSig); - }); + $this->processManager->fork( + function (SharedMemory $sharedMem) use (&$testSig) { + $sharedMem->signal($testSig); + } + ); $this->processManager->wait(); @@ -138,7 +155,10 @@ public function testPreviousSignalHandler(): void $this->assertEquals(3, $sigOrig); $this->assertEquals(2, $sigNew); - $this->processManager->getEventDispatcher()->removeSignalListener($testSig, $newSigHandler); + $this->processManager->getEventDispatcher()->removeSignalListener( + $testSig, + $newSigHandler + ); $currSigHandler = pcntl_signal_get_handler($testSig); $this->assertNotEquals($origSigHandler, $currSigHandler); @@ -150,7 +170,10 @@ public function testPreviousSignalHandler(): void $this->assertEquals(4, $sigOrig); $this->assertEquals(2, $sigNew); - $this->processManager->addListener($testSig, $newSigHandler); + $this->processManager->getEventDispatcher()->addSignalListener( + $testSig, + $newSigHandler + ); $currSigHandler = pcntl_signal_get_handler($testSig); $this->assertNotEquals($origSigHandler, $currSigHandler); @@ -162,9 +185,15 @@ public function testPreviousSignalHandler(): void $this->assertEquals(5, $sigOrig); $this->assertEquals(3, $sigNew); - $this->processManager->getEventDispatcher()->removeSignalHandlerWrappers(); + $this->processManager + ->getEventDispatcher() + ->removeSignalHandlerWrappers() + ; - $this->assertEquals($origSigHandler, pcntl_signal_get_handler($testSig)); + $this->assertEquals( + $origSigHandler, + pcntl_signal_get_handler($testSig) + ); $this->assertEquals(5, $sigOrig); $this->assertEquals(3, $sigNew); @@ -182,9 +211,12 @@ public function testSignalHandlerInstallFailure(): void ); $this->expectExceptionCode(22); - $this->processManager->addListener(255, function () { - // Do nothing. - }); + $this->processManager->getEventDispatcher()->addSignalListener( + 255, + function () { + // Do nothing. + } + ); } public function testSignalHandlerInstallErrorHandling(): void @@ -195,12 +227,17 @@ public function testSignalHandlerInstallErrorHandling(): void ); $this->expectExceptionCode(22); - $reflection = new ReflectionObject($this->processManager->getEventDispatcher()); + $reflection = new ReflectionObject( + $this->processManager->getEventDispatcher() + ); $method = $reflection->getMethod('setSignalHandler'); $method->setAccessible(true); - $method->invokeArgs($this->processManager->getEventDispatcher(), [255, function () { - // Do nothing. - }]); + $method->invokeArgs( + $this->processManager->getEventDispatcher(), + [255, function () { + // Do nothing. + }] + ); } } From 6142fd018367050805ee487967e39bd7f29ad140 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sun, 26 Jan 2020 11:27:31 +0100 Subject: [PATCH 12/16] #5: Add composer all script --- composer.json | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/composer.json b/composer.json index 5de3884..75803e9 100644 --- a/composer.json +++ b/composer.json @@ -81,6 +81,13 @@ "sort-packages": true }, "scripts": { + "all": [ + "composer cs", + "composer csf", + "composer static", + "composer test", + "composer coverage" + ], "cs": "vendor/bin/phpcs --standard=PSR12 src/ tests/", "csf": "vendor/bin/php-cs-fixer fix", "static": "vendor/bin/phpstan analyse", From 2a4000bc8927165d8d63f0f62e7949b9eb2ca246 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sun, 26 Jan 2020 11:38:07 +0100 Subject: [PATCH 13/16] #5: Run travis for different event dispatcher versions --- .phan/config.php | 1 + .travis.yml | 7 ++++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.phan/config.php b/.phan/config.php index d6e1ccc..a6bae4f 100644 --- a/.phan/config.php +++ b/.phan/config.php @@ -385,6 +385,7 @@ 'directory_list' => [ 'src/Spork', 'vendor/symfony/event-dispatcher', + 'vendor/symfony/event-dispatcher-contracts', 'vendor/symfony/var-dumper', ], diff --git a/.travis.yml b/.travis.yml index b0508af..e0d4268 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,13 +12,18 @@ php: - 7.4 - nightly +env: + matrix: + - PREFER_LOWEST="--prefer-lowest" + - PREFER_LOWEST="" + matrix: fast_finish: true allow_failures: - php: nightly install: - - composer install --no-progress --no-scripts --no-suggest --no-interaction + - composer update --no-progress --no-scripts --no-suggest --no-interaction --prefer-stable $PREFER_LOWEST script: - vendor/bin/phpcs --standard=PSR12 src/ tests/ From 3d58ddac3729a39eb3aba69d9aa837dba3bf41e1 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sun, 26 Jan 2020 19:42:31 +0100 Subject: [PATCH 14/16] #7: Fix incorrect shared memory access --- src/Spork/SharedMemory.php | 99 +++++++++++++++++++++++++++----------- 1 file changed, 71 insertions(+), 28 deletions(-) diff --git a/src/Spork/SharedMemory.php b/src/Spork/SharedMemory.php index e7650a1..d8ab150 100644 --- a/src/Spork/SharedMemory.php +++ b/src/Spork/SharedMemory.php @@ -1,14 +1,16 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork; use Spork\Exception\ProcessControlException; @@ -49,55 +51,69 @@ public function __construct($pid = null, $signal = null) /** * Reads all messages from shared memory. * - * @return array An array of messages + * @return mixed Any data that has been serialized into the shared memory. */ public function receive() { - if ($shmId = @shmop_open($this->pid, 'a', 0, 0)) { - $serializedMessages = shmop_read($shmId, 0, shmop_size($shmId)); - shmop_delete($shmId); - shmop_close($shmId); + if (false === ($shmId = @shmop_open($this->pid, 'a', 0, 0))) { + return []; + } + + /** @var string|false $sharedMemory */ + $sharedMemory = shmop_read($shmId, 0, 0); + if (false === $sharedMemory) { + throw new ProcessControlException(sprintf( + 'Not able to read from shared memory segment for PID: %d', + $this->pid + )); + } - return unserialize($serializedMessages); + if (false === shmop_delete($shmId)) { + throw new ProcessControlException(sprintf( + 'Not able to delete shared memory segment for PID: %d', + $this->pid + )); } + shmop_close($shmId); - return []; + return unserialize($this->strFromMem($sharedMemory)); } /** * Writes a message to the shared memory. * - * @param mixed $message The message to send - * @param int|null|false $signal The signal to send afterward. Null to use - * @param int $pause The number of microseconds to pause after signalling + * @param mixed $message The message to send. + * @param int|null|false $signal The signal to send afterward. Null to use. + * @param int $pause The number of microseconds to pause after + * signalling. */ public function send($message, $signal = null, $pause = 500) { - $messageArray = []; - - if ($shmId = @shmop_open($this->pid, 'a', 0, 0)) { - // Read any existing messages in shared memory - $readMessage = shmop_read($shmId, 0, shmop_size($shmId)); - $messageArray[] = unserialize($readMessage); - shmop_delete($shmId); - shmop_close($shmId); + $messages = $this->receive(); + if (!is_array($message)) { + $messages = [ + $messages, + ]; } // Add the current message to the end of the array, and serialize it - $messageArray[] = $message; - $serializedMessage = serialize($messageArray); + $messages[] = $message; + $serializedMsgs = serialize($messages); + $terminatedMsgs = $this->strToMem($serializedMsgs); + $termMsgsLen = strlen($terminatedMsgs); // Write new serialized message to shared memory - $shmId = shmop_open($this->pid, 'c', 0644, strlen($serializedMessage)); + $shmId = shmop_open($this->pid, 'c', 0644, $termMsgsLen); if (!is_resource($shmId)) { throw new ProcessControlException(sprintf( - 'Not able to create shared memory segment for PID: %s', + 'Not able to create shared memory segment for PID: %d', + $this->pid + )); + } elseif (shmop_write($shmId, $terminatedMsgs, 0) !== $termMsgsLen) { + throw new ProcessControlException(sprintf( + 'Not able to write to shared memory segment for PID: %d.', $this->pid )); - } elseif (shmop_write($shmId, $serializedMessage, 0) !== strlen($serializedMessage)) { - throw new ProcessControlException( - 'Not able to write message to shared memory segment.' - ); } if (false === $signal) { @@ -118,4 +134,31 @@ public function signal($signal) return posix_kill($pid, $signal); } + + /** + * Prepares a string to be stored in memory by appending a terminating zero. + * + * @param string $string String to be prepared. + * @return string String safe to be directly put in memory. + */ + private function strToMem(string &$string): string + { + return "{$string}\0"; + } + + /** + * Reads a string from memory by stopping at the first terminating zero. + * + * @param string $rawString String from memory. + * @return string String ending without the first terminating zero. + */ + private function strFromMem(string &$rawString): string + { + $pos = strpos($rawString, "\0"); + if (false === $pos) { + return $rawString; + } + + return substr($rawString, 0, $pos); + } } From 925fc36c81817ccc8bac3b9b0805f261c89fc399 Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sat, 1 Feb 2020 21:34:33 +0100 Subject: [PATCH 15/16] #7: Do not execute parent's shutdown function in child process --- src/Spork/ProcessManager.php | 14 ++++++++++---- tests/Spork/ProcessManagerTest.php | 6 ++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/Spork/ProcessManager.php b/src/Spork/ProcessManager.php index 24cb836..b7190d8 100644 --- a/src/Spork/ProcessManager.php +++ b/src/Spork/ProcessManager.php @@ -1,14 +1,16 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork; use InvalidArgumentException; @@ -95,6 +97,7 @@ public function fork($callable) } if (0 === $pid) { + $currPid = posix_getpid(); // reset the list of child processes $this->forks = []; @@ -103,8 +106,11 @@ public function fork($callable) $message = new ExitMessage(); // phone home on shutdown - register_shutdown_function(function () use ($shm, $message): void { - $status = null; + register_shutdown_function(function () use ($currPid, $shm, $message): void { + // Do not execute this function in child processes. + if ($currPid !== posix_getpid()) { + return; + } try { $shm->send($message, false); diff --git a/tests/Spork/ProcessManagerTest.php b/tests/Spork/ProcessManagerTest.php index 784c6a7..434fd47 100644 --- a/tests/Spork/ProcessManagerTest.php +++ b/tests/Spork/ProcessManagerTest.php @@ -1,14 +1,16 @@ * * For the full copyright and license information, please view the LICENSE * file that was distributed with this source code. */ +declare(strict_types=1); + namespace Spork; use Exception; From 2f9bc7dc3825f9c3770d2afe4548f2578bd0b35b Mon Sep 17 00:00:00 2001 From: Petr Levtonov Date: Sat, 1 Feb 2020 22:39:27 +0100 Subject: [PATCH 16/16] Update CHANGELOG.md --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 625e899..27f5c88 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [2.0.0] - 2019-11-29 + +### Changed + +- #5: Preserve and restore previous signal handler. Refactored event dispatcher. + +### Removed + +- #5: Removed the method `addListener` from the `ProcessManager` class. Add + signal/normal listeners through the event dispatcher on the process manager. + +### Fixed + +- #7: Fixed missing null terminator handling on shared memory blocks. +- #8: Fixed parent's shutdown function being executed in child processes. + ## [1.0.0] - 2019-11-29 ### Added @@ -38,5 +54,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Added serializable objects for exit and error messages. [Unreleased]: https://github.com/TheLevti/spork/compare/0.3.0...HEAD +[2.0.0]: https://github.com/TheLevti/spork/releases/2.0.0 [1.0.0]: https://github.com/TheLevti/spork/releases/1.0.0 [0.3.0]: https://github.com/TheLevti/spork/releases/0.3.0