Skip to content
Merged
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
22 changes: 22 additions & 0 deletions cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"log"
"os"
"os/exec"
"runtime"
"testing"

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

// Regression test for https://github.com/php/frankenphp/issues/1902. A
// long-running CLI script that installs pcntl_signal handlers must
// receive its own signals reliably
func TestExecuteScriptCLISignals(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("pcntl is not available on Windows")
}
if _, err := os.Stat("internal/testcli/testcli"); err != nil {
t.Skip("internal/testcli/testcli has not been compiled, run `cd internal/testcli/ && go build`")
}

cmd := exec.Command("internal/testcli/testcli", "testdata/command-pcntl.php")
stdoutStderr, err := cmd.CombinedOutput()
var exitError *exec.ExitError
if errors.As(err, &exitError) && exitError.ExitCode() == 2 {
t.Skipf("pcntl/posix not available: %s", stdoutStderr)
}
assert.NoError(t, err, "output: %s", stdoutStderr)
assert.Contains(t, string(stdoutStderr), "ok")
}

func ExampleExecuteScriptCLI() {
if len(os.Args) <= 1 {
log.Println("Usage: my-program script.php")
Expand Down
33 changes: 33 additions & 0 deletions frankenphp.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,33 @@ static void frankenphp_fork_child(void) {
static void frankenphp_register_atfork(void) {
pthread_atfork(frankenphp_fork_prepare, NULL, frankenphp_fork_child);
}

/* pcntl signals delivered to a Go M segfault on PCNTL_G (no TSRM there)
* Block these in a constructor so Go's schedinit captures
* the mask and every M inherits it; execute_script_cli unblocks on its own
* pthread. Caddy's `signal.Notify` keeps working via `runtime.ensureSigM`.
* Limited to async-notify signals: Go's minitSignalMask re-unblocks anything
* flagged _SigKill/_SigThrow/_SigUnblock on every M anyway. */
static void frankenphp_fill_cli_signal_set(sigset_t *s) {
sigemptyset(s);
#ifdef SIGUSR1
sigaddset(s, SIGUSR1);
#endif
#ifdef SIGUSR2
sigaddset(s, SIGUSR2);
#endif
#ifdef SIGALRM
sigaddset(s, SIGALRM);
#endif
}

__attribute__((constructor)) static void frankenphp_libpreinit(void) {
sigset_t set;
frankenphp_fill_cli_signal_set(&set);
/* Single-threaded at this point (constructors run before Go's runtime),
* so sigprocmask is sufficient and portable. */
sigprocmask(SIG_BLOCK, &set, NULL);
}
Comment on lines +203 to +207

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking the CLI-related signals process-wide in the constructor (via sigprocmask) also blocks termination signals like SIGTERM/SIGINT for all Go threads by default.

Yes, that's the point.

Please consider adding an explicit mitigation for library embedders (e.g. documented requirement, or a Go-side bootstrap that ensures SIGTERM/SIGINT remain deliverable even when no signal.Notify is used).

I'm not sure how realistic it is to actually hit this issue. The paths that Sloppilot mentioned should get stuck don't actually get stuck.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like Go unblocks sigkillable signals anyway, so this was effectively a no-op for the signals sloppilot flagged: https://github.com/golang/go/blob/1952e618b834bda60fb9efff0fc0af46b38e110d/src/runtime/signal_unix.go#L1356

#endif

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

#ifndef PHP_WIN32
sigset_t cli_signals;
frankenphp_fill_cli_signal_set(&cli_signals);
pthread_sigmask(SIG_UNBLOCK, &cli_signals, NULL);
#endif

/*
* The SAPI name "cli" is hardcoded into too many programs... let's usurp it.
*/
Expand Down
47 changes: 47 additions & 0 deletions testdata/command-pcntl.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

// Long-running CLI script that uses pcntl signals,
// simulating queue processors like Laravel Horizon or Symfony Messenger.

foreach (['pcntl_async_signals', 'pcntl_signal', 'pcntl_alarm', 'posix_kill'] as $fn) {
if (!function_exists($fn)) {
fwrite(STDERR, "missing $fn (pcntl/posix not fully loaded)\n");
exit(2);
}
}

pcntl_async_signals(true);

$received = ['SIGUSR1' => 0, 'SIGUSR2' => 0, 'SIGALRM' => 0];

pcntl_signal(SIGUSR1, function () use (&$received) { $received['SIGUSR1']++; });
pcntl_signal(SIGUSR2, function () use (&$received) { $received['SIGUSR2']++; });
pcntl_signal(SIGALRM, function () use (&$received) { $received['SIGALRM']++; });

$pid = getmypid();
$attempts = 0;
$deadline = microtime(true) + 1.5;
while (microtime(true) < $deadline) {
posix_kill($pid, SIGUSR1);
posix_kill($pid, SIGUSR2);
$attempts += 2;
usleep(500);
}

pcntl_alarm(1);
$alarmDeadline = microtime(true) + 1.5;
while (microtime(true) < $alarmDeadline && $received['SIGALRM'] === 0) {
usleep(1000);
}

if ($received['SIGUSR1'] === 0 || $received['SIGUSR2'] === 0) {
fwrite(STDERR, "missed user signals: " . json_encode($received) . " of $attempts attempts\n");
exit(1);
}
if ($received['SIGALRM'] === 0) {
fwrite(STDERR, "missed SIGALRM from pcntl_alarm\n");
exit(1);
}

echo "ok\n";
exit(0);
Loading