fix: don't segfault when the PHP CLI is executed while FrankenPHP is running

cli.go

@@ -2,11 +2,47 @@ 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. 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
+	}
+
 	// Ensure extensions are registered before CLI execution
 	registerExtensions()
 
@@ -19,7 +55,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()
 

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';"))
+}

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.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")

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))
 }