diff --git a/src/ExtEventLoop.php b/src/ExtEventLoop.php index 9db1de9d..53bbc974 100644 --- a/src/ExtEventLoop.php +++ b/src/ExtEventLoop.php @@ -41,27 +41,7 @@ public function __construct(EventBaseConfig $config = null) $this->eventBase = new EventBase($config); $this->futureTickQueue = new FutureTickQueue(); $this->timerEvents = new SplObjectStorage(); - - $this->signals = new SignalsHandler( - $this, - function ($signal) { - $this->signalEvents[$signal] = Event::signal($this->eventBase, $signal, $f = function () use ($signal, &$f) { - $this->signals->call($signal); - // Ensure there are two copies of the callable around until it has been executed. - // For more information see: https://bugs.php.net/bug.php?id=62452 - // Only an issue for PHP 5, this hack can be removed once PHP 5 support has been dropped. - $g = $f; - $f = $g; - }); - $this->signalEvents[$signal]->add(); - }, - function ($signal) { - if ($this->signals->count($signal) === 0) { - $this->signalEvents[$signal]->free(); - unset($this->signalEvents[$signal]); - } - } - ); + $this->signals = new SignalsHandler(); $this->createTimerCallback(); $this->createStreamCallback(); @@ -167,11 +147,21 @@ public function futureTick($listener) public function addSignal($signal, $listener) { $this->signals->add($signal, $listener); + + if (!isset($this->signalEvents[$signal])) { + $this->signalEvents[$signal] = Event::signal($this->eventBase, $signal, array($this->signals, 'call')); + $this->signalEvents[$signal]->add(); + } } public function removeSignal($signal, $listener) { $this->signals->remove($signal, $listener); + + if (isset($this->signalEvents[$signal]) && $this->signals->count($signal) === 0) { + $this->signalEvents[$signal]->free(); + unset($this->signalEvents[$signal]); + } } public function run() @@ -184,7 +174,7 @@ public function run() $flags = EventBase::LOOP_ONCE; if (!$this->running || !$this->futureTickQueue->isEmpty()) { $flags |= EventBase::LOOP_NONBLOCK; - } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count()) { + } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count() && $this->signals->isEmpty()) { break; } diff --git a/src/ExtLibevLoop.php b/src/ExtLibevLoop.php index 58e75b9f..587bb044 100644 --- a/src/ExtLibevLoop.php +++ b/src/ExtLibevLoop.php @@ -39,28 +39,7 @@ public function __construct() $this->loop = new EventLoop(); $this->futureTickQueue = new FutureTickQueue(); $this->timerEvents = new SplObjectStorage(); - - $this->signals = new SignalsHandler( - $this, - function ($signal) { - $this->signalEvents[$signal] = new SignalEvent($f = function () use ($signal, &$f) { - $this->signals->call($signal); - // Ensure there are two copies of the callable around until it has been executed. - // For more information see: https://bugs.php.net/bug.php?id=62452 - // Only an issue for PHP 5, this hack can be removed once PHP 5 support has been dropped. - $g = $f; - $f = $g; - }, $signal); - $this->loop->add($this->signalEvents[$signal]); - }, - function ($signal) { - if ($this->signals->count($signal) === 0) { - $this->signalEvents[$signal]->stop(); - $this->loop->remove($this->signalEvents[$signal]); - unset($this->signalEvents[$signal]); - } - } - ); + $this->signals = new SignalsHandler(); } public function addReadStream($stream, $listener) @@ -167,11 +146,24 @@ public function futureTick($listener) public function addSignal($signal, $listener) { $this->signals->add($signal, $listener); + + if (!isset($this->signalEvents[$signal])) { + $this->signalEvents[$signal] = new SignalEvent(function () use ($signal) { + $this->signals->call($signal); + }, $signal); + $this->loop->add($this->signalEvents[$signal]); + } } public function removeSignal($signal, $listener) { $this->signals->remove($signal, $listener); + + if (isset($this->signalEvents[$signal]) && $this->signals->count($signal) === 0) { + $this->signalEvents[$signal]->stop(); + $this->loop->remove($this->signalEvents[$signal]); + unset($this->signalEvents[$signal]); + } } public function run() @@ -184,7 +176,7 @@ public function run() $flags = EventLoop::RUN_ONCE; if (!$this->running || !$this->futureTickQueue->isEmpty()) { $flags |= EventLoop::RUN_NOWAIT; - } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count()) { + } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count() && $this->signals->isEmpty()) { break; } diff --git a/src/ExtLibeventLoop.php b/src/ExtLibeventLoop.php index db1bb65a..da181a28 100644 --- a/src/ExtLibeventLoop.php +++ b/src/ExtLibeventLoop.php @@ -55,30 +55,7 @@ public function __construct() $this->eventBase = event_base_new(); $this->futureTickQueue = new FutureTickQueue(); $this->timerEvents = new SplObjectStorage(); - - $this->signals = new SignalsHandler( - $this, - function ($signal) { - $this->signalEvents[$signal] = event_new(); - event_set($this->signalEvents[$signal], $signal, EV_PERSIST | EV_SIGNAL, $f = function () use ($signal, &$f) { - $this->signals->call($signal); - // Ensure there are two copies of the callable around until it has been executed. - // For more information see: https://bugs.php.net/bug.php?id=62452 - // Only an issue for PHP 5, this hack can be removed once PHP 5 support has been dropped. - $g = $f; - $f = $g; - }); - event_base_set($this->signalEvents[$signal], $this->eventBase); - event_add($this->signalEvents[$signal]); - }, - function ($signal) { - if ($this->signals->count($signal) === 0) { - event_del($this->signalEvents[$signal]); - event_free($this->signalEvents[$signal]); - unset($this->signalEvents[$signal]); - } - } - ); + $this->signals = new SignalsHandler(); $this->createTimerCallback(); $this->createStreamCallback(); @@ -185,11 +162,24 @@ public function futureTick($listener) public function addSignal($signal, $listener) { $this->signals->add($signal, $listener); + + if (!isset($this->signalEvents[$signal])) { + $this->signalEvents[$signal] = event_new(); + event_set($this->signalEvents[$signal], $signal, EV_PERSIST | EV_SIGNAL, array($this->signals, 'call')); + event_base_set($this->signalEvents[$signal], $this->eventBase); + event_add($this->signalEvents[$signal]); + } } public function removeSignal($signal, $listener) { $this->signals->remove($signal, $listener); + + if (isset($this->signalEvents[$signal]) && $this->signals->count($signal) === 0) { + event_del($this->signalEvents[$signal]); + event_free($this->signalEvents[$signal]); + unset($this->signalEvents[$signal]); + } } public function run() @@ -202,7 +192,7 @@ public function run() $flags = EVLOOP_ONCE; if (!$this->running || !$this->futureTickQueue->isEmpty()) { $flags |= EVLOOP_NONBLOCK; - } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count()) { + } elseif (!$this->readEvents && !$this->writeEvents && !$this->timerEvents->count() && $this->signals->isEmpty()) { break; } diff --git a/src/SignalsHandler.php b/src/SignalsHandler.php index cbf1bf31..2614e03c 100644 --- a/src/SignalsHandler.php +++ b/src/SignalsHandler.php @@ -7,41 +7,12 @@ */ final class SignalsHandler { - private $loop; - private $timer; private $signals = []; - private $on; - private $off; - - public function __construct(LoopInterface $loop, $on, $off) - { - $this->loop = $loop; - $this->on = $on; - $this->off = $off; - } - - public function __destruct() - { - $off = $this->off; - foreach ($this->signals as $signal => $listeners) { - $off($signal); - } - } public function add($signal, $listener) { - if (empty($this->signals) && $this->timer === null) { - /** - * Timer to keep the loop alive as long as there are any signal handlers registered - */ - $this->timer = $this->loop->addPeriodicTimer(300, function () {}); - } - if (!isset($this->signals[$signal])) { $this->signals[$signal] = []; - - $on = $this->on; - $on($signal); } if (in_array($listener, $this->signals[$signal])) { @@ -62,14 +33,6 @@ public function remove($signal, $listener) if (isset($this->signals[$signal]) && \count($this->signals[$signal]) === 0) { unset($this->signals[$signal]); - - $off = $this->off; - $off($signal); - } - - if (empty($this->signals) && $this->timer instanceof TimerInterface) { - $this->loop->cancelTimer($this->timer); - $this->timer = null; } } @@ -92,4 +55,9 @@ public function count($signal) return \count($this->signals[$signal]); } + + public function isEmpty() + { + return !$this->signals; + } } diff --git a/src/StreamSelectLoop.php b/src/StreamSelectLoop.php index ed672ce7..55bb1b8a 100644 --- a/src/StreamSelectLoop.php +++ b/src/StreamSelectLoop.php @@ -69,24 +69,7 @@ public function __construct() $this->futureTickQueue = new FutureTickQueue(); $this->timers = new Timers(); $this->pcntl = extension_loaded('pcntl'); - $this->signals = new SignalsHandler( - $this, - function ($signal) { - \pcntl_signal($signal, $f = function ($signal) use (&$f) { - $this->signals->call($signal); - // Ensure there are two copies of the callable around until it has been executed. - // For more information see: https://bugs.php.net/bug.php?id=62452 - // Only an issue for PHP 5, this hack can be removed once PHP 5 support has been dropped. - $g = $f; - $f = $g; - }); - }, - function ($signal) { - if ($this->signals->count($signal) === 0) { - \pcntl_signal($signal, SIG_DFL); - } - } - ); + $this->signals = new SignalsHandler(); } public function addReadStream($stream, $listener) @@ -163,12 +146,25 @@ public function addSignal($signal, $listener) throw new \BadMethodCallException('Event loop feature "signals" isn\'t supported by the "StreamSelectLoop"'); } + $first = $this->signals->count($signal) === 0; $this->signals->add($signal, $listener); + + if ($first) { + \pcntl_signal($signal, array($this->signals, 'call')); + } } public function removeSignal($signal, $listener) { + if (!$this->signals->count($signal)) { + return; + } + $this->signals->remove($signal, $listener); + + if ($this->signals->count($signal) === 0) { + \pcntl_signal($signal, SIG_DFL); + } } public function run() @@ -197,8 +193,8 @@ public function run() $timeout = $timeout > PHP_INT_MAX ? PHP_INT_MAX : (int)$timeout; } - // The only possible event is stream activity, so wait forever ... - } elseif ($this->readStreams || $this->writeStreams) { + // The only possible event is stream or signal activity, so wait forever ... + } elseif ($this->readStreams || $this->writeStreams || !$this->signals->isEmpty()) { $timeout = null; // There's nothing left to do ... diff --git a/tests/AbstractLoopTest.php b/tests/AbstractLoopTest.php index 296bdc7e..6cac6eb1 100644 --- a/tests/AbstractLoopTest.php +++ b/tests/AbstractLoopTest.php @@ -475,6 +475,12 @@ function () { $this->loop->run(); } + public function testRemoveSignalNotRegisteredIsNoOp() + { + $this->loop->removeSignal(SIGINT, function () { }); + $this->assertTrue(true); + } + public function testSignal() { if (!function_exists('posix_kill') || !function_exists('posix_getpid')) { diff --git a/tests/SignalsHandlerTest.php b/tests/SignalsHandlerTest.php index 04f37151..f8b7df3d 100644 --- a/tests/SignalsHandlerTest.php +++ b/tests/SignalsHandlerTest.php @@ -2,7 +2,6 @@ namespace React\Tests\EventLoop; -use React\EventLoop\Factory; use React\EventLoop\SignalsHandler; final class SignalsHandlerTest extends TestCase @@ -10,83 +9,47 @@ final class SignalsHandlerTest extends TestCase public function testEmittedEventsAndCallHandling() { $callCount = 0; - $onCount = 0; - $offCount = 0; $func = function () use (&$callCount) { $callCount++; }; - $signals = new SignalsHandler( - Factory::create(), - function () use (&$onCount) { - $onCount++; - }, - function () use (&$offCount) { - $offCount++; - } - ); + $signals = new SignalsHandler(); $this->assertSame(0, $callCount); - $this->assertSame(0, $onCount); - $this->assertSame(0, $offCount); $signals->add(SIGUSR1, $func); $this->assertSame(0, $callCount); - $this->assertSame(1, $onCount); - $this->assertSame(0, $offCount); $signals->add(SIGUSR1, $func); $this->assertSame(0, $callCount); - $this->assertSame(1, $onCount); - $this->assertSame(0, $offCount); $signals->add(SIGUSR1, $func); $this->assertSame(0, $callCount); - $this->assertSame(1, $onCount); - $this->assertSame(0, $offCount); $signals->call(SIGUSR1); $this->assertSame(1, $callCount); - $this->assertSame(1, $onCount); - $this->assertSame(0, $offCount); $signals->add(SIGUSR2, $func); $this->assertSame(1, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(0, $offCount); $signals->add(SIGUSR2, $func); $this->assertSame(1, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(0, $offCount); $signals->call(SIGUSR2); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(0, $offCount); $signals->remove(SIGUSR2, $func); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(1, $offCount); $signals->remove(SIGUSR2, $func); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(1, $offCount); $signals->call(SIGUSR2); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(1, $offCount); $signals->remove(SIGUSR1, $func); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(2, $offCount); $signals->call(SIGUSR1); $this->assertSame(2, $callCount); - $this->assertSame(2, $onCount); - $this->assertSame(2, $offCount); } }