Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide siginfo to signal callback if possible #79

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
"ext-json": "*",
"phpunit/phpunit": "^9",
"jetbrains/phpstorm-stubs": "^2019.3",
"psalm/phar": "^4.7"
"psalm/phar": "^5"
},
"autoload": {
"psr-4": {
Expand Down
7 changes: 7 additions & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@
</errorLevel>
</StringIncrement>

<UnsupportedPropertyReferenceUsage>
<errorLevel type="suppress">
<directory name="examples"/>
<directory name="src"/>
</errorLevel>
</UnsupportedPropertyReferenceUsage>

<RedundantConditionGivenDocblockType>
<errorLevel type="suppress">
<directory name="src"/>
Expand Down
32 changes: 24 additions & 8 deletions src/EventLoop/Driver/StreamSelectDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Revolt\EventLoop\Internal\AbstractDriver;
use Revolt\EventLoop\Internal\DriverCallback;
use Revolt\EventLoop\Internal\SignalCallback;
use Revolt\EventLoop\Internal\SignalCallbackExtra;
use Revolt\EventLoop\Internal\StreamReadableCallback;
use Revolt\EventLoop\Internal\StreamWritableCallback;
use Revolt\EventLoop\Internal\TimerCallback;
Expand All @@ -31,10 +32,10 @@ final class StreamSelectDriver extends AbstractDriver

private readonly TimerQueue $timerQueue;

/** @var array<int, array<string, SignalCallback>> */
/** @var array<int, array<string, SignalCallback|SignalCallbackExtra>> */
private array $signalCallbacks = [];

/** @var \SplQueue<int> */
/** @var \SplQueue<list{int, mixed}> */
private readonly \SplQueue $signalQueue;

private bool $signalHandling;
Expand Down Expand Up @@ -101,6 +102,18 @@ public function onSignal(int $signal, \Closure $closure): string
return parent::onSignal($signal, $closure);
}

/**
* @throws UnsupportedFeatureException If the pcntl extension is not available.
*/
public function onSignalWithInfo(int $signal, \Closure $closure): string
{
if (!$this->signalHandling) {
throw new UnsupportedFeatureException("Signal handling requires the pcntl extension");
}

return parent::onSignalWithInfo($signal, $closure);
}

public function getHandle(): mixed
{
return null;
Expand All @@ -120,9 +133,12 @@ protected function dispatch(bool $blocking): void
\pcntl_signal_dispatch();

while (!$this->signalQueue->isEmpty()) {
$signal = $this->signalQueue->dequeue();
[$signal, $siginfo] = $this->signalQueue->dequeue();

foreach ($this->signalCallbacks[$signal] as $callback) {
if ($callback instanceof SignalCallbackExtra) {
$callback->siginfo = $siginfo;
}
$this->enqueueCallback($callback);
}

Expand Down Expand Up @@ -160,7 +176,7 @@ protected function activate(array $callbacks): void
$this->writeStreams[$streamId] = $callback->stream;
} elseif ($callback instanceof TimerCallback) {
$this->timerQueue->insert($callback);
} elseif ($callback instanceof SignalCallback) {
} elseif ($callback instanceof SignalCallback || $callback instanceof SignalCallbackExtra) {
if (!isset($this->signalCallbacks[$callback->signal])) {
\set_error_handler(static function (int $errno, string $errstr): bool {
throw new UnsupportedFeatureException(
Expand Down Expand Up @@ -203,7 +219,7 @@ protected function deactivate(DriverCallback $callback): void
}
} elseif ($callback instanceof TimerCallback) {
$this->timerQueue->remove($callback);
} elseif ($callback instanceof SignalCallback) {
} elseif ($callback instanceof SignalCallback || $callback instanceof SignalCallbackExtra) {
if (isset($this->signalCallbacks[$callback->signal])) {
unset($this->signalCallbacks[$callback->signal][$callback->id]);

Expand Down Expand Up @@ -304,7 +320,7 @@ private function selectStreams(array $read, array $write, float $timeout): void
}

if ($timeout > 0) { // Sleep until next timer expires.
/** @psalm-var positive-int $timeout */
/** @psalm-suppress ArgumentTypeCoercion $timeout is always > 0, even if there is no psalm type to represent a positive float. */
\usleep((int) ($timeout * 1_000_000));
}
}
Expand All @@ -325,9 +341,9 @@ private function getTimeout(): float
return $expiration > 0 ? $expiration : 0.0;
}

private function handleSignal(int $signal): void
private function handleSignal(int $signal, mixed $siginfo): void
{
// Queue signals, so we don't suspend inside pcntl_signal_dispatch, which disables signals while it runs
$this->signalQueue->enqueue($signal);
$this->signalQueue->enqueue([$signal, $siginfo]);
}
}
4 changes: 2 additions & 2 deletions src/EventLoop/Driver/TracingDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ private function getCancelTrace(string $callbackId): string
/**
* Formats a stacktrace obtained via `debug_backtrace()`.
*
* @param array<array{file?: string, line: int, type?: string, class?: class-string, function: string}> $trace
* @param list<array{args?: list<mixed>, class?: class-string, file?: string, function: string, line?: int, object?: object, type?: string}> $trace
* Output of `debug_backtrace()`.
*
* @return string Formatted stacktrace.
Expand All @@ -259,7 +259,7 @@ private function formatStacktrace(array $trace): string
return \implode("\n", \array_map(static function ($e, $i) {
$line = "#{$i} ";

if (isset($e["file"])) {
if (isset($e["file"]) && isset($e['line'])) {
$line .= "{$e['file']}:{$e['line']} ";
}

Expand Down
16 changes: 16 additions & 0 deletions src/EventLoop/Internal/AbstractDriver.php
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,16 @@ public function onSignal(int $signal, \Closure $closure): string
return $signalCallback->id;
}

protected function onSignalWithInfo(int $signal, \Closure $closure): string
{
$signalCallback = new SignalCallbackExtra($this->nextId++, $closure, $signal, null);

$this->callbacks[$signalCallback->id] = $signalCallback;
$this->enableQueue[$signalCallback->id] = $signalCallback;

return $signalCallback->id;
}

public function enable(string $callbackId): string
{
if (!isset($this->callbacks[$callbackId])) {
Expand Down Expand Up @@ -517,6 +527,7 @@ private function createLoopFiber(): void
// Invoke microtasks if we have some
$this->invokeCallbacks();

/** @var bool $this->stopped */
while (!$this->stopped) {
if ($this->interrupt) {
$this->invokeInterrupt();
Expand Down Expand Up @@ -574,6 +585,11 @@ private function createCallbackFiber(): void
$callback->id,
$callback->signal
),
$callback instanceof SignalCallbackExtra => ($callback->closure)(
$callback->id,
$callback->signal,
$callback->siginfo
),
default => ($callback->closure)($callback->id),
};

Expand Down
18 changes: 18 additions & 0 deletions src/EventLoop/Internal/SignalCallbackExtra.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Revolt\EventLoop\Internal;

/** @internal */
final class SignalCallbackExtra extends DriverCallback
{
public function __construct(
string $id,
\Closure $closure,
public readonly int $signal,
public mixed $siginfo
) {
parent::__construct($id, $closure);
}
}
11 changes: 8 additions & 3 deletions test/Driver/StreamSelectDriverTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,13 @@ public function testAsyncSignals(): void
\pcntl_async_signals(true);

try {
$this->start(function (Driver $loop) use (&$invoked, &$callbackId) {
$callbackId = $loop->onSignal(SIGUSR1, function () use (&$invoked) {
$this->start(function (StreamSelectDriver $loop) use (&$invoked, &$callbackId) {
$callbackId = $loop->onSignalWithInfo(SIGUSR1, function ($cId, $sig, $siginfo) use (&$callbackId, &$invoked) {
$this->assertEquals($callbackId, $cId);
$this->assertEquals(SIGUSR1, $sig);
$this->assertEquals(SIGUSR1, $siginfo['signo']);
$this->assertEquals(\getmypid(), $siginfo['pid']);
$this->assertEquals(\getmyuid(), $siginfo['uid']);
$invoked = true;
});

Expand Down Expand Up @@ -121,7 +126,7 @@ public function testSignalDuringStreamSelectIgnored(): void

$sockets = \stream_socket_pair(STREAM_PF_UNIX, STREAM_SOCK_STREAM, STREAM_IPPROTO_IP);

$this->start(function (Driver $loop) use ($sockets, &$signalCallbackId) {
$this->start(function (StreamSelectDriver $loop) use ($sockets, &$signalCallbackId) {
$socketCallbackIds = [
$loop->onReadable($sockets[0], function () {
// nothing
Expand Down