diff --git a/src/Util/PcntlTimeout.php b/src/Util/PcntlTimeout.php index c066ee9..88705f7 100644 --- a/src/Util/PcntlTimeout.php +++ b/src/Util/PcntlTimeout.php @@ -53,34 +53,39 @@ public function __construct(int $timeout) * * @return T * + * @throws \Throwable * @throws DeadlineException Running the code hit the deadline * @throws LockAcquireException Installing the timeout failed */ public function timeBoxed(callable $code) { - $existingHandler = pcntl_signal_get_handler(\SIGALRM); + if (pcntl_alarm($this->timeout) !== 0) { + throw new LockAcquireException('Existing process alarm is not supported'); + } + + $origSignalHandler = pcntl_signal_get_handler(\SIGALRM); - $signal = pcntl_signal(\SIGALRM, function (): void { + $timeout = $this->timeout; + $signalHandlerFx = static function () use ($timeout): void { throw new DeadlineException(sprintf( 'Timebox hit deadline of %d seconds', - $this->timeout + $timeout )); - }); - if (!$signal) { - throw new LockAcquireException('Could not install signal'); - } + }; - $oldAlarm = pcntl_alarm($this->timeout); - if ($oldAlarm !== 0) { - throw new LockAcquireException('Existing alarm was not expected'); + if (!pcntl_signal(\SIGALRM, $signalHandlerFx)) { + throw new LockAcquireException('Failed to install signal handler'); } try { return $code(); } finally { pcntl_alarm(0); - pcntl_signal_dispatch(); - pcntl_signal(\SIGALRM, $existingHandler); + try { + pcntl_signal_dispatch(); + } finally { + pcntl_signal(\SIGALRM, $origSignalHandler); + } } } diff --git a/tests/Util/LoopTest.php b/tests/Util/LoopTest.php index afeff11..df08c90 100644 --- a/tests/Util/LoopTest.php +++ b/tests/Util/LoopTest.php @@ -46,7 +46,6 @@ public function testInvalidAcquireTimeout(float $acquireTimeout): void $this->expectException(\InvalidArgumentException::class); $this->expectExceptionMessage('The lock acquire timeout must be greater than or equal to 0.0 (' . LockUtil::getInstance()->formatTimeout($acquireTimeout) . ' was given)'); - $loop->execute(static function () { self::fail(); }, $acquireTimeout); @@ -78,10 +77,10 @@ public function testExecutionWithinAcquireTimeout(): void public function testExecutionWithinAcquireTimeoutWithoutCallingEnd(): void { + $loop = new Loop(); + $this->expectException(LockAcquireTimeoutException::class); $this->expectExceptionMessage('Lock acquire timeout of 0.5 seconds has been exceeded'); - - $loop = new Loop(); $loop->execute(static function () { usleep(10 * 1000); }, 0.5); @@ -102,10 +101,10 @@ public function testExceedAcquireTimeoutIsAcceptableIfEndWasCalled(): void public function testExceedAcquireTimeoutWithoutCallingEnd(): void { + $loop = new Loop(); + $this->expectException(LockAcquireTimeoutException::class); $this->expectExceptionMessage('Lock acquire timeout of 0.5 seconds has been exceeded'); - - $loop = new Loop(); $loop->execute(static function () { usleep(501 * 1000); }, 0.5); @@ -113,9 +112,9 @@ public function testExceedAcquireTimeoutWithoutCallingEnd(): void public function testExceptionStopsIteration(): void { - $this->expectException(\DomainException::class); - $loop = new Loop(); + + $this->expectException(\DomainException::class); $loop->execute(static function () { throw new \DomainException(); }, 1); diff --git a/tests/Util/PcntlTimeoutTest.php b/tests/Util/PcntlTimeoutTest.php index 775c037..e4980d4 100644 --- a/tests/Util/PcntlTimeoutTest.php +++ b/tests/Util/PcntlTimeoutTest.php @@ -21,10 +21,9 @@ class PcntlTimeoutTest extends TestCase */ public function testShouldTimeout(): void { - $this->expectException(DeadlineException::class); - $timeout = new PcntlTimeout(1); + $this->expectException(DeadlineException::class); $timeout->timeBoxed(static function () { sleep(2); }); @@ -44,22 +43,37 @@ public function testShouldNotTimeout(): void self::assertSame(42, $result); } + /** + * Thrown exceptions from the subject code should be rethrown. + */ + public function testShouldThrowException(): void + { + $timeout = new PcntlTimeout(1); + + $this->expectException(\DomainException::class); + $timeout->timeBoxed(static function () { + throw new \DomainException(); + }); + } + /** * When a previous scheduled alarm exists, it should fail. */ public function testShouldFailOnExistingAlarm(): void { - $this->expectException(LockAcquireException::class); - + $origSignalHandler = pcntl_signal_get_handler(\SIGALRM); try { pcntl_alarm(1); $timeout = new PcntlTimeout(1); + $this->expectException(LockAcquireException::class); + $this->expectExceptionMessage('Existing process alarm is not supported'); $timeout->timeBoxed(static function () { sleep(1); }); } finally { pcntl_alarm(0); + self::assertSame($origSignalHandler, pcntl_signal_get_handler(\SIGALRM)); } } @@ -74,4 +88,21 @@ public function testShouldResetAlarmWhenNotTimeout(): void self::assertSame(0, pcntl_alarm(0)); } + + /** + * After not timing out and throwing an exception, there should be no alarm scheduled. + */ + public function testShouldResetAlarmWhenNotTimeoutAndException(): void + { + $timeout = new PcntlTimeout(3); + + $this->expectException(\DomainException::class); + try { + $timeout->timeBoxed(static function () { + throw new \DomainException(); + }); + } finally { + self::assertSame(0, pcntl_alarm(0)); + } + } }