From 09d4e91b51c2bdeb5635d2710c14700bb20a8a38 Mon Sep 17 00:00:00 2001 From: cyppe Date: Fri, 7 Aug 2026 08:27:43 +0200 Subject: [PATCH 1/2] fix: exit after PID 1 PHP thread segfault --- cli_test.go | 46 ++++++++++++++++++++++++++++++ frankenphp.c | 39 +++++++++++++++++++++++++ internal/testcli/main.go | 5 ++++ internal/testcli/segfault_linux.go | 27 ++++++++++++++++++ internal/testcli/segfault_other.go | 5 ++++ 5 files changed, 122 insertions(+) create mode 100644 internal/testcli/segfault_linux.go create mode 100644 internal/testcli/segfault_other.go diff --git a/cli_test.go b/cli_test.go index 964bb49907..1301ca28a1 100644 --- a/cli_test.go +++ b/cli_test.go @@ -1,12 +1,14 @@ package frankenphp_test import ( + "context" "errors" "log" "os" "os/exec" "runtime" "testing" + "time" "github.com/dunglas/frankenphp" "github.com/stretchr/testify/assert" @@ -67,6 +69,50 @@ func TestExecuteScriptCLISignals(t *testing.T) { assert.Contains(t, string(stdoutStderr), "ok") } +// Regression test for https://github.com/php/frankenphp/issues/2558. When a +// C-created thread segfaults while FrankenPHP is PID 1, the process must +// exit instead of looping forever in Go's runtime.raisebadsignal. +func TestCThreadSegfaultAsPID1(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("PID namespaces are only available on Linux") + } + if _, err := os.Stat("internal/testcli/testcli"); err != nil { + t.Skip("internal/testcli/testcli has not been compiled, run `cd internal/testcli/ && go build`") + } + if _, err := exec.LookPath("unshare"); err != nil { + t.Skip("unshare is not available") + } + + probe := exec.Command("unshare", "--user", "--map-root-user", "--pid", "--fork", "true") + if output, err := probe.CombinedOutput(); err != nil { + t.Skipf("unprivileged PID namespaces are not available: %s", output) + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext( + ctx, + "unshare", + "--user", + "--map-root-user", + "--pid", + "--fork", + "--kill-child=KILL", + "internal/testcli/testcli", + "--segfault", + ) + output, err := cmd.CombinedOutput() + if errors.Is(ctx.Err(), context.DeadlineExceeded) { + t.Fatalf("FrankenPHP did not exit after the PHP thread segfaulted: %s", output) + } + + var exitError *exec.ExitError + if !errors.As(err, &exitError) { + t.Fatalf("expected FrankenPHP to exit with an error, got %v: %s", err, output) + } + assert.Equal(t, 2, exitError.ExitCode(), "output: %s", output) +} + func ExampleExecuteScriptCLI() { if len(os.Args) <= 1 { log.Println("Usage: my-program script.php") diff --git a/frankenphp.c b/frankenphp.c index a47b6d80a7..9d705e527f 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -222,7 +222,46 @@ static void frankenphp_fill_cli_signal_set(sigset_t *s) { #endif } +#if defined(__linux__) +/* Linux ignores default-action signals re-raised by PID 1 from inside its + * namespace. Go's runtime preserves handlers installed before it starts and + * forwards synchronous faults from C-created threads to them, so this turns a + * PHP-thread SIGSEGV into a process exit instead of the raisebadsignal loop. + * See https://github.com/php/frankenphp/issues/2558 and + * https://github.com/golang/go/issues/59569. */ +static void frankenphp_pid1_sigsegv_handler(int sig, siginfo_t *info, + void *context) { + (void)sig; + (void)info; + (void)context; + _exit(2); +} + +static void frankenphp_install_pid1_sigsegv_handler(void) { + if (getpid() != 1) { + return; + } + + struct sigaction previous; + if (sigaction(SIGSEGV, NULL, &previous) != 0 || + previous.sa_handler != SIG_DFL) { + return; + } + + struct sigaction action; + memset(&action, 0, sizeof(action)); + action.sa_sigaction = frankenphp_pid1_sigsegv_handler; + sigemptyset(&action.sa_mask); + action.sa_flags = SA_SIGINFO | SA_ONSTACK; + sigaction(SIGSEGV, &action, NULL); +} +#endif + __attribute__((constructor)) static void frankenphp_libpreinit(void) { +#if defined(__linux__) + frankenphp_install_pid1_sigsegv_handler(); +#endif + sigset_t set; frankenphp_fill_cli_signal_set(&set); /* Single-threaded at this point (constructors run before Go's runtime), diff --git a/internal/testcli/main.go b/internal/testcli/main.go index c03c836c4d..3cf3e604c0 100644 --- a/internal/testcli/main.go +++ b/internal/testcli/main.go @@ -8,6 +8,11 @@ import ( ) func main() { + if len(os.Args) == 2 && os.Args[1] == "--segfault" { + triggerSIGSEGVOnCThread() + os.Exit(3) + } + if len(os.Args) <= 1 { log.Println("Usage: testcli script.php") os.Exit(1) diff --git a/internal/testcli/segfault_linux.go b/internal/testcli/segfault_linux.go new file mode 100644 index 0000000000..6ec065d976 --- /dev/null +++ b/internal/testcli/segfault_linux.go @@ -0,0 +1,27 @@ +//go:build linux + +package main + +/* +#include +#include + +static void *segfault(void *unused) { + (void)unused; + *(volatile int *)0 = 1; + return NULL; +} + +static void trigger_sigsegv_on_c_thread(void) { + pthread_t thread; + if (pthread_create(&thread, NULL, segfault, NULL) != 0) { + _exit(3); + } + pthread_join(thread, NULL); +} +*/ +import "C" + +func triggerSIGSEGVOnCThread() { + C.trigger_sigsegv_on_c_thread() +} diff --git a/internal/testcli/segfault_other.go b/internal/testcli/segfault_other.go new file mode 100644 index 0000000000..5ba06fac4d --- /dev/null +++ b/internal/testcli/segfault_other.go @@ -0,0 +1,5 @@ +//go:build !linux + +package main + +func triggerSIGSEGVOnCThread() {} From f2e0c1c93090f58f5013081f5b7532e0089e13d4 Mon Sep 17 00:00:00 2001 From: cyppe Date: Fri, 7 Aug 2026 09:05:56 +0200 Subject: [PATCH 2/2] test: enforce PID 1 regression in CI --- .github/workflows/tests.yaml | 2 + cli_test.go | 88 +++++++++++++++++++++++++++++++----- 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index 8b0759f7a6..45d9604b21 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -60,6 +60,8 @@ jobs: - name: Install gotestsum run: go install gotest.tools/gotestsum@latest - name: Run library tests + env: + FRANKENPHP_TEST_PID_NAMESPACE_WITH_SUDO: "1" run: gotestsum -- -race ./... - name: Run Caddy module tests working-directory: caddy/ diff --git a/cli_test.go b/cli_test.go index 1301ca28a1..5a9ac678ba 100644 --- a/cli_test.go +++ b/cli_test.go @@ -6,6 +6,7 @@ import ( "log" "os" "os/exec" + "path/filepath" "runtime" "testing" "time" @@ -76,31 +77,84 @@ func TestCThreadSegfaultAsPID1(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("PID namespaces are only available on Linux") } - if _, err := os.Stat("internal/testcli/testcli"); err != nil { + useSudo := os.Getenv("FRANKENPHP_TEST_PID_NAMESPACE_WITH_SUDO") == "1" + testCLIPath, err := filepath.Abs("internal/testcli/testcli") + if err != nil { + t.Fatal(err) + } + if _, err := os.Stat(testCLIPath); err != nil { + if useSudo { + t.Fatalf("internal/testcli/testcli must be compiled for the PID namespace test: %v", err) + } t.Skip("internal/testcli/testcli has not been compiled, run `cd internal/testcli/ && go build`") } - if _, err := exec.LookPath("unshare"); err != nil { + unsharePath, err := exec.LookPath("unshare") + if err != nil { + if useSudo { + t.Fatal("unshare is required for the PID namespace test") + } t.Skip("unshare is not available") } - - probe := exec.Command("unshare", "--user", "--map-root-user", "--pid", "--fork", "true") - if output, err := probe.CombinedOutput(); err != nil { - t.Skipf("unprivileged PID namespaces are not available: %s", output) + truePath, err := exec.LookPath("true") + if err != nil { + t.Fatal("true is required for the PID namespace test probe") } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - defer cancel() - cmd := exec.CommandContext( - ctx, - "unshare", + command := unsharePath + args := []string{ "--user", "--map-root-user", "--pid", "--fork", "--kill-child=KILL", - "internal/testcli/testcli", + testCLIPath, "--segfault", + } + rootlessOutput, rootlessErr := runPIDNamespaceProbe( + unsharePath, + "--user", + "--map-root-user", + "--pid", + "--fork", + "--kill-child=KILL", + truePath, ) + if rootlessErr != nil { + if !useSudo { + t.Skipf("unprivileged PID namespaces are not available: %v: %s", rootlessErr, rootlessOutput) + } + + sudoPath, sudoErr := exec.LookPath("sudo") + if sudoErr != nil { + t.Fatal("sudo was requested for the PID namespace test but was not found") + } + sudoOutput, sudoErr := runPIDNamespaceProbe( + sudoPath, + "-n", + unsharePath, + "--pid", + "--fork", + "--kill-child=KILL", + truePath, + ) + if sudoErr != nil { + t.Fatalf("failed to create a PID namespace with sudo: %v: %s", sudoErr, sudoOutput) + } + command = sudoPath + args = []string{ + "-n", + unsharePath, + "--pid", + "--fork", + "--kill-child=KILL", + testCLIPath, + "--segfault", + } + } + + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, command, args...) output, err := cmd.CombinedOutput() if errors.Is(ctx.Err(), context.DeadlineExceeded) { t.Fatalf("FrankenPHP did not exit after the PHP thread segfaulted: %s", output) @@ -113,6 +167,16 @@ func TestCThreadSegfaultAsPID1(t *testing.T) { assert.Equal(t, 2, exitError.ExitCode(), "output: %s", output) } +func runPIDNamespaceProbe(command string, args ...string) ([]byte, error) { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + output, err := exec.CommandContext(ctx, command, args...).CombinedOutput() + if ctx.Err() != nil { + return output, ctx.Err() + } + return output, err +} + func ExampleExecuteScriptCLI() { if len(os.Args) <= 1 { log.Println("Usage: my-program script.php")