diff --git a/frankenphp.c b/frankenphp.c index 077ab5a187..31996daca3 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1810,6 +1810,33 @@ void frankenphp_add_to_prepared_env(char *name, size_t name_len, char *val, zend_hash_str_update(prepared_env, name, name_len, &zv); } +#if defined(ZTS) && PHP_VERSION_ID < 80700 +/* Destroys the persistent resources of the thread (pfsockopen(), persistent + * PDO connections...) while the thread and its modules are still intact. + * Since PHP 8.4 (php-src 5941cdaa), ts_free_thread() no longer does it: + * zend_shutdown() destroys the lists of the threads still registered in TSRM, + * which a thread that exits earlier is not, so every thread restart (reboot, + * max_requests) leaked them. Up to PHP 8.3, ts_free_thread() destroys the + * list itself, but a reboot with a persistent stream then aborts with a + * corrupted heap. PHP 8.7 destroys the list in ts_free_thread() again + * (php-src 0a98b979), the same way as here. */ +static void frankenphp_destroy_thread_persistent_list(void) { + dtor_func_t destructor = EG(persistent_list).pDestructor; + volatile bool completed = false; + + do { + zend_try { + zend_hash_graceful_reverse_destroy(&EG(persistent_list)); + completed = true; + } + zend_end_try(); + } while (!completed); + + /* An empty list again, so that destroying it a second time is harmless */ + zend_hash_init(&EG(persistent_list), 8, NULL, destructor, 1); +} +#endif + static void *php_thread(void *arg) { thread_index = (uintptr_t)arg; char thread_name[16] = {0}; @@ -1957,6 +1984,9 @@ static void *php_thread(void *arg) { /* free all global PHP memory reserved for this thread */ #ifdef ZTS +#if PHP_VERSION_ID < 80700 + frankenphp_destroy_thread_persistent_list(); +#endif ts_free_thread(); #endif diff --git a/persistent_resources_test.go b/persistent_resources_test.go new file mode 100644 index 0000000000..fdf7afd604 --- /dev/null +++ b/persistent_resources_test.go @@ -0,0 +1,110 @@ +package frankenphp_test + +import ( + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + "time" + + "github.com/dunglas/frankenphp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// socketCounter accepts the sockets opened by testdata/persistent-socket.php +// and counts how many of them PHP closed again. +type socketCounter struct { + listener net.Listener + opened atomic.Int32 + closed atomic.Int32 +} + +func newSocketCounter(t *testing.T) *socketCounter { + t.Helper() + + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = listener.Close() }) + + counter := &socketCounter{listener: listener} + go func() { + for { + connection, err := listener.Accept() + if err != nil { + return + } + counter.opened.Add(1) + + go func() { + defer func() { _ = connection.Close() }() + // returns once PHP closes its end of the socket + _, _ = io.Copy(io.Discard, connection) + counter.closed.Add(1) + }() + } + }() + + return counter +} + +func (counter *socketCounter) url() string { + return fmt.Sprintf("http://example.com/persistent-socket.php?port=%d", counter.listener.Addr().(*net.TCPAddr).Port) +} + +func (counter *socketCounter) assertCounts(t *testing.T, opened int32, closed int32) { + t.Helper() + + assert.EventuallyWithT(t, func(c *assert.CollectT) { + assert.Equal(c, opened, counter.opened.Load(), "persistent sockets opened") + assert.Equal(c, closed, counter.closed.Load(), "persistent sockets closed") + }, 5*time.Second, 10*time.Millisecond) +} + +// A PHP thread must destroy its persistent resources (pfsockopen(), persistent +// PDO connections...) before it exits: since PHP 8.4, ts_free_thread() no +// longer does it, and up to PHP 8.3 a reboot then aborts with a corrupted heap +// for persistent streams. +func TestPersistentResourcesAreDestroyedOnReboot(t *testing.T) { + sockets := newSocketCounter(t) + + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + body, _ := testGet(sockets.url(), handler, t) + require.Equal(t, "ok", body) + sockets.assertCounts(t, 1, 0) + + frankenphp.RestartWorkers() + + body, _ = testGet(sockets.url(), handler, t) + require.Equal(t, "ok", body) + sockets.assertCounts(t, 2, 1) + }, &testOptions{ + nbParallelRequests: 1, + initOpts: []frankenphp.Option{frankenphp.WithNumThreads(1)}, + }) +} + +func TestPersistentResourcesAreDestroyedOnMaxRequests(t *testing.T) { + const maxRequests = 2 + const totalRequests = 10 + sockets := newSocketCounter(t) + + runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) { + for range totalRequests { + body, _ := testGet(sockets.url(), handler, t) + require.Equal(t, "ok", body) + } + + // each thread reuses its socket until it restarts after maxRequests + sockets.assertCounts(t, totalRequests/maxRequests, totalRequests/maxRequests) + }, &testOptions{ + nbParallelRequests: 1, + initOpts: []frankenphp.Option{ + frankenphp.WithNumThreads(1), + frankenphp.WithMaxRequests(maxRequests), + }, + }) +} diff --git a/testdata/persistent-socket.php b/testdata/persistent-socket.php new file mode 100644 index 0000000000..0baa723e74 --- /dev/null +++ b/testdata/persistent-socket.php @@ -0,0 +1,12 @@ +