From 44972c0a20b9da05029d7772be28587c80692061 Mon Sep 17 00:00:00 2001 From: Aguinaldo Tupy <44652991+aguinaldotupy@users.noreply.github.com> Date: Wed, 28 Oct 2026 16:00:39 -0300 Subject: [PATCH] fix: keep custom extensions registered after PHP restarts register_internal_extensions() freed the module list after the first php_module_startup(), so every later startup registered no custom extension: their functions, classes and constants were gone after a main thread reboot (file watcher, the admin `workers/restart` endpoint, `opcache_reset()`) and after `Shutdown()` and `Init()`, which Caddy runs on config reloads. Keep the C copy of the list for the process lifetime and make register_extensions() install the hook only once. PHP destroys the module registry and resets `module_started` on every module shutdown, and registers its own static modules the same way on each startup. Fixes #2572 Co-authored-by: ousamabenyounes Co-Authored-By: Claude Opus 5.5 --- frankenphp.c | 11 ++++--- internal/testext/extensions.c | 51 +++++++++++++++++++++++++---- internal/testext/exttest.go | 35 ++++++++++++++------ internal/testext/testdata/index.php | 6 +++- 4 files changed, 81 insertions(+), 22 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index 4066845349..98cfe72add 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -2124,20 +2124,23 @@ int register_internal_extensions(void) { return FAILURE; } + /* modules is kept for the process lifetime: every php_module_startup() + * (main thread reboots, a new Init()) registers the modules again */ for (int i = 0; i < modules_len; i++) { if (zend_register_internal_module(modules[i]) == NULL) { return FAILURE; } } - pefree(modules, 1); - modules = NULL; - modules_len = 0; - return SUCCESS; } void register_extensions(zend_module_entry **m, int len) { + /* install the hook only once, or it would become its own predecessor */ + if (modules != NULL || len <= 0) { + return; + } + modules = pemalloc(len * sizeof(*modules), 1); memcpy(modules, m, len * sizeof(*modules)); modules_len = len; diff --git a/internal/testext/extensions.c b/internal/testext/extensions.c index 0731a8c29e..d8df4274d0 100644 --- a/internal/testext/extensions.c +++ b/internal/testext/extensions.c @@ -86,13 +86,50 @@ zend_module_entry module1_entry = {STANDARD_MODULE_HEADER, "0.1.0", STANDARD_MODULE_PROPERTIES}; +/* counts MINIT calls for the whole process: each PHP startup, including the + * main thread reboots, must register the module again */ +static zend_long ext2_minit_calls = 0; + +ZEND_BEGIN_MODULE_GLOBALS(ext2) +zend_long answer; +ZEND_END_MODULE_GLOBALS(ext2) + +ZEND_DECLARE_MODULE_GLOBALS(ext2) + +#define EXT2_G(v) ZEND_MODULE_GLOBALS_ACCESSOR(ext2, v) + +static PHP_GINIT_FUNCTION(ext2) { ext2_globals->answer = 42; } + +static PHP_MINIT_FUNCTION(ext2) { + REGISTER_LONG_CONSTANT("EXT2_MINIT_CALLS", ++ext2_minit_calls, + CONST_PERSISTENT); + + return SUCCESS; +} + +static PHP_FUNCTION(ext2_answer) { + ZEND_PARSE_PARAMETERS_NONE(); + + RETURN_LONG(EXT2_G(answer)); +} + +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ext2_answer, 0, 0, IS_LONG, 0) +ZEND_END_ARG_INFO() + +static const zend_function_entry ext2_functions[] = { + ZEND_FE(ext2_answer, arginfo_ext2_answer) ZEND_FE_END}; + zend_module_entry module2_entry = {STANDARD_MODULE_HEADER, "ext2", - NULL, /* Functions */ - NULL, /* MINIT */ - NULL, /* MSHUTDOWN */ - NULL, /* RINIT */ - NULL, /* RSHUTDOWN */ - NULL, /* MINFO */ + ext2_functions, /* Functions */ + PHP_MINIT(ext2), /* MINIT */ + NULL, /* MSHUTDOWN */ + NULL, /* RINIT */ + NULL, /* RSHUTDOWN */ + NULL, /* MINFO */ "0.1.0", - STANDARD_MODULE_PROPERTIES}; + PHP_MODULE_GLOBALS(ext2), + PHP_GINIT(ext2), + NULL, /* GSHUTDOWN */ + NULL, /* post deactivate */ + STANDARD_MODULE_PROPERTIES_EX}; diff --git a/internal/testext/exttest.go b/internal/testext/exttest.go index 1704d3142d..cce4468da1 100644 --- a/internal/testext/exttest.go +++ b/internal/testext/exttest.go @@ -12,6 +12,7 @@ package testext // #include "extension.h" import "C" import ( + "fmt" "io" "net/http/httptest" "runtime" @@ -58,17 +59,31 @@ func testRegisterExtension(t *testing.T) { _ = *p }) - req := httptest.NewRequest("GET", "http://example.com/index.php", nil) - w := httptest.NewRecorder() + assertExtensionsLoaded(t, 1) + + // rebooting the main thread (file watcher, admin restart endpoint, + // opcache_reset()) starts the PHP modules again + frankenphp.RestartWorkers() + assertExtensionsLoaded(t, 2) + + // so does a new Init(), which Caddy runs on every config reload + frankenphp.Shutdown() + require.NoError(t, frankenphp.Init()) + assertExtensionsLoaded(t, 3) +} + +// assertExtensionsLoaded checks that both test modules are registered and +// that ext2's MINIT ran minitCalls times since the process started +func assertExtensionsLoaded(t *testing.T, minitCalls int) { + t.Helper() - req, err = frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot("./testdata", false)) - assert.NoError(t, err) + req := httptest.NewRequest("GET", "http://example.com/index.php", nil) + req, err := frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot("./testdata", false)) + require.NoError(t, err) - err = frankenphp.ServeHTTP(w, req) - assert.NoError(t, err) + w := httptest.NewRecorder() + require.NoError(t, frankenphp.ServeHTTP(w, req)) - resp := w.Result() - body, _ := io.ReadAll(resp.Body) - assert.Contains(t, string(body), "ext1") - assert.Contains(t, string(body), "ext2") + body, _ := io.ReadAll(w.Result().Body) + assert.Equal(t, fmt.Sprintf("ext1: loaded\next2: loaded\next2_answer: 42\nEXT2_MINIT_CALLS: %d\n", minitCalls), string(body)) } diff --git a/internal/testext/testdata/index.php b/internal/testext/testdata/index.php index 96c7dc6588..ffc84e3272 100644 --- a/internal/testext/testdata/index.php +++ b/internal/testext/testdata/index.php @@ -1,3 +1,7 @@