Skip to content

Commit

Permalink
Timeout should reset SIGALRM handler to SIG_DFL
Browse files Browse the repository at this point in the history
This was the documented behaviour, but tests confirming that were
missing. After implementing the tests, it turned out that the
implementation violated that contract.
  • Loading branch information
malkusch authored and mvorisek committed Dec 3, 2024
1 parent c24a6b9 commit 163ceda
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 2 deletions.
7 changes: 5 additions & 2 deletions src/util/PcntlTimeout.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,11 @@ public function timeBoxed(callable $code)
return call_user_func($code);
} finally {
pcntl_alarm(0);
pcntl_signal_dispatch();
pcntl_signal(SIGALRM, SIG_DFL);
try {
pcntl_signal_dispatch();
} finally {
pcntl_signal(SIGALRM, SIG_DFL);
}
}
}

Expand Down
137 changes: 137 additions & 0 deletions tests/util/PcntlTimeoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace malkusch\lock\util;

use malkusch\lock\exception\TimeoutException;

/**
* Tests for PcntlTimeout
*
Expand Down Expand Up @@ -29,6 +31,23 @@ public function shouldTimeout()
});
}

/**
* A long running system call should be interrupted,
* any thrown exception should be ignored.
*
* @test
* @expectedException malkusch\lock\exception\TimeoutException
*/
public function shouldTimeoutAndIgnoreException()

Check failure on line 41 in tests/util/PcntlTimeoutTest.php

View workflow job for this annotation

GitHub Actions / Smoke (latest, StaticAnalysis)

Method malkusch\lock\util\PcntlTimeoutTest::shouldTimeoutAndIgnoreException() has no return type specified.
{
$timeout = new PcntlTimeout(1);

$timeout->timeBoxed(function () {
sleep(2);
throw new \DomainException("Swallow me");
});
}

/**
* A short running system call should complete its execution
*
Expand All @@ -45,6 +64,21 @@ public function shouldNotTimeout()
$this->assertEquals(42, $result);

Check failure on line 64 in tests/util/PcntlTimeoutTest.php

View workflow job for this annotation

GitHub Actions / Smoke (latest, StaticAnalysis)

Call to an undefined method malkusch\lock\util\PcntlTimeoutTest::assertEquals().
}

/**
* Thrown exceptions from the subject code should be rethrown
*
* @test
* @expectedException \DomainException
*/
public function shouldThrowException()

Check failure on line 73 in tests/util/PcntlTimeoutTest.php

View workflow job for this annotation

GitHub Actions / Smoke (latest, StaticAnalysis)

Method malkusch\lock\util\PcntlTimeoutTest::shouldThrowException() has no return type specified.
{
$timeout = new PcntlTimeout(1);

$timeout->timeBoxed(function () {
throw new \DomainException();
});
}

/**
* When a previous scheduled alarm exists, it should fail
*
Expand Down Expand Up @@ -79,4 +113,107 @@ public function shouldResetAlarmWhenNotTimeout()

$this->assertEquals(0, pcntl_alarm(0));
}

/**
* After not timing out and throwing an exception, there should be no alarm scheduled
*
* @test
*/
public function shouldResetAlarmWhenNotTimeoutAndException()
{
$timeout = new PcntlTimeout(3);

try {
$timeout->timeBoxed(function () {
throw new \DomainException();
});
} catch (\DomainException $e) {
// expected
}

$this->assertEquals(0, pcntl_alarm(0));
}

/**
* After a timeout, the default signal handler should be installed
*
* @test
*/
public function shouldResetToDefaultHandlerAfterTimeout()
{
$pid = pcntl_fork();
if ($pid == -1) {
$this->fail('could not fork');
} elseif ($pid === 0) {
try {
$timeout = new PcntlTimeout(1);
$timeout->timeBoxed(function () {
sleep(3);
});
} catch (TimeoutException $e) {
// expected
}

pcntl_alarm(1);
sleep(3);
exit;
}
pcntl_wait($status);

$this->assertEquals(SIGALRM, pcntl_wtermsig($status));
}

/**
* After no timeout, the default signal handler should be installed
*
* @test
*/
public function shouldResetToDefaultHandlerAfterNoTimeout()
{
$pid = pcntl_fork();
if ($pid == -1) {
$this->fail('could not fork');
} elseif ($pid === 0) {
$timeout = new PcntlTimeout(3);
$timeout->timeBoxed(function () {
return 42;
});

pcntl_alarm(1);
sleep(3);
exit;
}
pcntl_wait($status);

$this->assertEquals(SIGALRM, pcntl_wtermsig($status));
}

/**
* After no timeout and throwing an exception, the default signal handler should be installed
*
* @test
*/
public function shouldResetToDefaultHandlerAfterNoTimeoutAndException()
{
$pid = pcntl_fork();
if ($pid == -1) {
$this->fail('could not fork');
} elseif ($pid === 0) {
try {
$timeout = new PcntlTimeout(3);
$timeout->timeBoxed(function () {
throw new \DomainException();
});
} catch (\DomainException $e) {
// expected
}

pcntl_alarm(1);
sleep(3);
exit;
}
pcntl_wait($status);

$this->assertEquals(SIGALRM, pcntl_wtermsig($status));
}
}

0 comments on commit 163ceda

Please sign in to comment.