From b0171d3be7326cfa7a5a3224fb6237d1e0bc11f5 Mon Sep 17 00:00:00 2001 From: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com> Date: Wed, 5 Aug 2026 23:51:59 +0000 Subject: [PATCH 1/3] fix: don't segfault when the PHP CLI is executed while FrankenPHP is running ExecuteScriptCLI and ExecutePHPCode start the embedded PHP CLI SAPI via php_embed_init(). When FrankenPHP is already running in the same process (Init has been called), booting the embedded SAPI on top of the running FrankenPHP SAPI corrupts the PHP engine and crashes the whole process with a segmentation fault. Refuse the call cleanly instead: when FrankenPHP is running, log an error and return a non-zero exit status rather than crashing. The standalone CLI path (no prior Init) is unchanged. Fixes #2342 Co-Authored-By: Claude Opus 4.8 (1M context) --- cli.go | 46 +++++++++++++++++++++++++++++++++++++++- cli_internal_test.go | 26 +++++++++++++++++++++++ cli_test.go | 24 +++++++++++++++++++++ internal/testcli/main.go | 13 ++++++++++++ 4 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 cli_internal_test.go diff --git a/cli.go b/cli.go index 96821a2392..cbd4f7c624 100644 --- a/cli.go +++ b/cli.go @@ -2,11 +2,45 @@ package frankenphp // #include "frankenphp.h" import "C" -import "unsafe" + +import ( + "log/slog" + "unsafe" +) + +// cliUsageErrorExitCode is the exit status returned by ExecuteScriptCLI and +// ExecutePHPCode when the call cannot be executed, for instance when FrankenPHP +// is already running in the current process. +const cliUsageErrorExitCode = 1 + +// refuseCLIWhileRunning reports whether a CLI execution must be refused because +// FrankenPHP is already running in this process. ExecuteScriptCLI/ExecutePHPCode +// start the embedded PHP CLI SAPI, and doing so on top of the already-started +// FrankenPHP SAPI corrupts the PHP engine and crashes the whole process with a +// segmentation fault. Run the PHP CLI in a dedicated process instead. +// See https://github.com/php/frankenphp/issues/2342. +func refuseCLIWhileRunning() bool { + if !isRunning { + return false + } + + globalLogger.LogAttrs(globalCtx, slog.LevelError, "the PHP CLI cannot be executed while FrankenPHP is running; run ExecuteScriptCLI/ExecutePHPCode in a dedicated process instead") + + return true +} // ExecuteScriptCLI executes the PHP script passed as parameter. // It returns the exit status code of the script. +// +// It must not be called while FrankenPHP is running in the same process (that is, +// after a successful [Init] and before [Shutdown]): the embedded PHP CLI SAPI +// cannot coexist with the running FrankenPHP SAPI. In that case the call is +// refused, an error is logged, and a non-zero exit status is returned. func ExecuteScriptCLI(script string, args []string) int { + if refuseCLIWhileRunning() { + return cliUsageErrorExitCode + } + // Ensure extensions are registered before CLI execution registerExtensions() @@ -19,7 +53,17 @@ func ExecuteScriptCLI(script string, args []string) int { return int(C.frankenphp_execute_script_cli(cScript, argc, (**C.char)(unsafe.Pointer(&argv[0])), false)) } +// ExecutePHPCode evaluates the PHP code passed as parameter. +// It returns the exit status code of the code. +// +// Like [ExecuteScriptCLI], it must not be called while FrankenPHP is running in +// the same process; in that case the call is refused, an error is logged, and a +// non-zero exit status is returned. func ExecutePHPCode(phpCode string) int { + if refuseCLIWhileRunning() { + return cliUsageErrorExitCode + } + // Ensure extensions are registered before CLI execution registerExtensions() diff --git a/cli_internal_test.go b/cli_internal_test.go new file mode 100644 index 0000000000..44f40fb999 --- /dev/null +++ b/cli_internal_test.go @@ -0,0 +1,26 @@ +package frankenphp + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Unit coverage for the guard added for +// https://github.com/php/frankenphp/issues/2342: the embedded PHP CLI SAPI must +// not be started while FrankenPHP is already running, otherwise the process +// crashes with a segmentation fault. +func TestRefuseCLIWhileRunning(t *testing.T) { + // When FrankenPHP is not running, the CLI helpers proceed normally. + assert.False(t, refuseCLIWhileRunning()) + + require.NoError(t, Init()) + defer Shutdown() + + // While running, the guard trips and the public entrypoints refuse cleanly + // with the usage exit code instead of crashing the process. + assert.True(t, refuseCLIWhileRunning()) + assert.Equal(t, cliUsageErrorExitCode, ExecuteScriptCLI("testdata/command.php", []string{"testdata/command.php"})) + assert.Equal(t, cliUsageErrorExitCode, ExecutePHPCode("echo 'noop';")) +} diff --git a/cli_test.go b/cli_test.go index 964bb49907..a74a16452d 100644 --- a/cli_test.go +++ b/cli_test.go @@ -10,6 +10,7 @@ import ( "github.com/dunglas/frankenphp" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestExecuteScriptCLI(t *testing.T) { @@ -67,6 +68,29 @@ func TestExecuteScriptCLISignals(t *testing.T) { assert.Contains(t, string(stdoutStderr), "ok") } +// Regression test for https://github.com/php/frankenphp/issues/2342. Calling +// ExecuteScriptCLI (or ExecutePHPCode) while FrankenPHP is already running in the +// same process used to boot the embedded PHP CLI SAPI on top of the running +// FrankenPHP SAPI, corrupting the engine and crashing the whole process with a +// segmentation fault. It must now be refused cleanly with a non-zero exit status. +func TestExecuteScriptCLIWhileRunning(t *testing.T) { + 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", "-init-first", "testdata/command.php") + stdoutStderr, err := cmd.CombinedOutput() + + var exitError *exec.ExitError + require.ErrorAs(t, err, &exitError, "output: %s", stdoutStderr) + + // The process must exit cleanly, not be killed by a signal (before the fix + // it crashed with SIGSEGV, reported by ProcessState.Exited() == false). + assert.True(t, exitError.ProcessState.Exited(), + "process was killed by a signal instead of exiting cleanly: %s\noutput: %s", exitError, stdoutStderr) + assert.Equal(t, 1, exitError.ExitCode(), "output: %s", stdoutStderr) +} + func ExampleExecuteScriptCLI() { if len(os.Args) <= 1 { log.Println("Usage: my-program script.php") diff --git a/internal/testcli/main.go b/internal/testcli/main.go index c03c836c4d..0a436eb7f4 100644 --- a/internal/testcli/main.go +++ b/internal/testcli/main.go @@ -17,5 +17,18 @@ func main() { os.Exit(frankenphp.ExecutePHPCode(os.Args[2])) } + // "-init-first script.php" starts FrankenPHP before running the CLI script, + // to exercise the guard against executing the PHP CLI while FrankenPHP is + // already running (https://github.com/php/frankenphp/issues/2342). + if len(os.Args) == 3 && os.Args[1] == "-init-first" { + if err := frankenphp.Init(); err != nil { + log.Fatalln(err) + } + + status := frankenphp.ExecuteScriptCLI(os.Args[2], os.Args[2:]) + frankenphp.Shutdown() + os.Exit(status) + } + os.Exit(frankenphp.ExecuteScriptCLI(os.Args[1], os.Args)) } From bd82d34873bead57de1411ff4162685dff66b5fb Mon Sep 17 00:00:00 2001 From: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com> Date: Thu, 6 Aug 2026 10:13:24 +0000 Subject: [PATCH 2/3] style: drop redundant embedded field from ExitError selector staticcheck QF1008: exec.ExitError embeds *os.ProcessState, so Exited() is promoted (as ExitCode() is already used on the next line). --- cli_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli_test.go b/cli_test.go index a74a16452d..5e1187a486 100644 --- a/cli_test.go +++ b/cli_test.go @@ -86,7 +86,7 @@ func TestExecuteScriptCLIWhileRunning(t *testing.T) { // The process must exit cleanly, not be killed by a signal (before the fix // it crashed with SIGSEGV, reported by ProcessState.Exited() == false). - assert.True(t, exitError.ProcessState.Exited(), + assert.True(t, exitError.Exited(), "process was killed by a signal instead of exiting cleanly: %s\noutput: %s", exitError, stdoutStderr) assert.Equal(t, 1, exitError.ExitCode(), "output: %s", stdoutStderr) } From 09762d8645d1e91c415715584a0347252edcc154 Mon Sep 17 00:00:00 2001 From: Ben Younes <2910651+ousamabenyounes@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:10:30 +0000 Subject: [PATCH 3/3] docs: spell out CLI-after-Init alternatives in ExecuteScriptCLI godoc The godoc already noted that ExecuteScriptCLI/ExecutePHPCode must not run while FrankenPHP is up. Spell out the actionable alternatives: run the CLI before Init(), after Shutdown(), or in a separate process (the same binary in "php-cli" mode), which gives a real CLI SAPI and process isolation. Refs #2342 Co-Authored-By: Claude Opus 4.8 (1M context) --- cli.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli.go b/cli.go index cbd4f7c624..f160f80c45 100644 --- a/cli.go +++ b/cli.go @@ -34,8 +34,10 @@ func refuseCLIWhileRunning() bool { // // It must not be called while FrankenPHP is running in the same process (that is, // after a successful [Init] and before [Shutdown]): the embedded PHP CLI SAPI -// cannot coexist with the running FrankenPHP SAPI. In that case the call is -// refused, an error is logged, and a non-zero exit status is returned. +// cannot coexist with the running FrankenPHP SAPI. Run it before [Init], after +// [Shutdown], or in a separate process (for example the same binary in "php-cli" +// mode) instead. While FrankenPHP is running the call is refused: an error is +// logged and a non-zero exit status is returned. func ExecuteScriptCLI(script string, args []string) int { if refuseCLIWhileRunning() { return cliUsageErrorExitCode