Skip to content

Commit 3bc35af

Browse files
authored
fix: pcntl signal reaching Go runtime causing long running cli scripts to get stuck (#2445)
1 parent 570bbf2 commit 3bc35af

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

cli_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"log"
66
"os"
77
"os/exec"
8+
"runtime"
89
"testing"
910

1011
"github.com/dunglas/frankenphp"
@@ -45,6 +46,27 @@ func TestExecuteCLICode(t *testing.T) {
4546
assert.Equal(t, stdoutStderrStr, `Hello World`)
4647
}
4748

49+
// Regression test for https://github.com/php/frankenphp/issues/1902. A
50+
// long-running CLI script that installs pcntl_signal handlers must
51+
// receive its own signals reliably
52+
func TestExecuteScriptCLISignals(t *testing.T) {
53+
if runtime.GOOS == "windows" {
54+
t.Skip("pcntl is not available on Windows")
55+
}
56+
if _, err := os.Stat("internal/testcli/testcli"); err != nil {
57+
t.Skip("internal/testcli/testcli has not been compiled, run `cd internal/testcli/ && go build`")
58+
}
59+
60+
cmd := exec.Command("internal/testcli/testcli", "testdata/command-pcntl.php")
61+
stdoutStderr, err := cmd.CombinedOutput()
62+
var exitError *exec.ExitError
63+
if errors.As(err, &exitError) && exitError.ExitCode() == 2 {
64+
t.Skipf("pcntl/posix not available: %s", stdoutStderr)
65+
}
66+
assert.NoError(t, err, "output: %s", stdoutStderr)
67+
assert.Contains(t, string(stdoutStderr), "ok")
68+
}
69+
4870
func ExampleExecuteScriptCLI() {
4971
if len(os.Args) <= 1 {
5072
log.Println("Usage: my-program script.php")

frankenphp.c

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,33 @@ static void frankenphp_fork_child(void) {
178178
static void frankenphp_register_atfork(void) {
179179
pthread_atfork(frankenphp_fork_prepare, NULL, frankenphp_fork_child);
180180
}
181+
182+
/* pcntl signals delivered to a Go M segfault on PCNTL_G (no TSRM there)
183+
* Block these in a constructor so Go's schedinit captures
184+
* the mask and every M inherits it; execute_script_cli unblocks on its own
185+
* pthread. Caddy's `signal.Notify` keeps working via `runtime.ensureSigM`.
186+
* Limited to async-notify signals: Go's minitSignalMask re-unblocks anything
187+
* flagged _SigKill/_SigThrow/_SigUnblock on every M anyway. */
188+
static void frankenphp_fill_cli_signal_set(sigset_t *s) {
189+
sigemptyset(s);
190+
#ifdef SIGUSR1
191+
sigaddset(s, SIGUSR1);
192+
#endif
193+
#ifdef SIGUSR2
194+
sigaddset(s, SIGUSR2);
195+
#endif
196+
#ifdef SIGALRM
197+
sigaddset(s, SIGALRM);
198+
#endif
199+
}
200+
201+
__attribute__((constructor)) static void frankenphp_libpreinit(void) {
202+
sigset_t set;
203+
frankenphp_fill_cli_signal_set(&set);
204+
/* Single-threaded at this point (constructors run before Go's runtime),
205+
* so sigprocmask is sufficient and portable. */
206+
sigprocmask(SIG_BLOCK, &set, NULL);
207+
}
181208
#endif
182209

183210
/* Best-effort force-kill for stuck PHP threads.
@@ -1689,6 +1716,12 @@ static void *execute_script_cli(void *arg) {
16891716
void *exit_status;
16901717
bool eval = (bool)arg;
16911718

1719+
#ifndef PHP_WIN32
1720+
sigset_t cli_signals;
1721+
frankenphp_fill_cli_signal_set(&cli_signals);
1722+
pthread_sigmask(SIG_UNBLOCK, &cli_signals, NULL);
1723+
#endif
1724+
16921725
/*
16931726
* The SAPI name "cli" is hardcoded into too many programs... let's usurp it.
16941727
*/

testdata/command-pcntl.php

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
// Long-running CLI script that uses pcntl signals,
4+
// simulating queue processors like Laravel Horizon or Symfony Messenger.
5+
6+
foreach (['pcntl_async_signals', 'pcntl_signal', 'pcntl_alarm', 'posix_kill'] as $fn) {
7+
if (!function_exists($fn)) {
8+
fwrite(STDERR, "missing $fn (pcntl/posix not fully loaded)\n");
9+
exit(2);
10+
}
11+
}
12+
13+
pcntl_async_signals(true);
14+
15+
$received = ['SIGUSR1' => 0, 'SIGUSR2' => 0, 'SIGALRM' => 0];
16+
17+
pcntl_signal(SIGUSR1, function () use (&$received) { $received['SIGUSR1']++; });
18+
pcntl_signal(SIGUSR2, function () use (&$received) { $received['SIGUSR2']++; });
19+
pcntl_signal(SIGALRM, function () use (&$received) { $received['SIGALRM']++; });
20+
21+
$pid = getmypid();
22+
$attempts = 0;
23+
$deadline = microtime(true) + 1.5;
24+
while (microtime(true) < $deadline) {
25+
posix_kill($pid, SIGUSR1);
26+
posix_kill($pid, SIGUSR2);
27+
$attempts += 2;
28+
usleep(500);
29+
}
30+
31+
pcntl_alarm(1);
32+
$alarmDeadline = microtime(true) + 1.5;
33+
while (microtime(true) < $alarmDeadline && $received['SIGALRM'] === 0) {
34+
usleep(1000);
35+
}
36+
37+
if ($received['SIGUSR1'] === 0 || $received['SIGUSR2'] === 0) {
38+
fwrite(STDERR, "missed user signals: " . json_encode($received) . " of $attempts attempts\n");
39+
exit(1);
40+
}
41+
if ($received['SIGALRM'] === 0) {
42+
fwrite(STDERR, "missed SIGALRM from pcntl_alarm\n");
43+
exit(1);
44+
}
45+
46+
echo "ok\n";
47+
exit(0);

0 commit comments

Comments
 (0)