fix: destroy persistent resources before a PHP thread exits

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
 

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

testdata/persistent-socket.php

@@ -0,0 +1,12 @@
+<?php
+
+// The socket stays open for the lifetime of the PHP thread, like a persistent PDO connection.
+$socket = pfsockopen('127.0.0.1', (int) $_GET['port'], $errorCode, $errorMessage, 5);
+if ($socket === false) {
+    http_response_code(500);
+    echo "pfsockopen failed: $errorMessage";
+
+    return;
+}
+
+echo 'ok';