Merge branch 'fix/opcache-safe-reset' of https://github.com/dunglas/frankenphp into fix/opcache-safe-reset

Author: AlliBalliBaba

caddy/caddy_test.go

@@ -3,7 +3,6 @@ package caddy_test
 import (
 	"bytes"
 	"fmt"
-	"math/rand/v2"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -1767,9 +1766,9 @@ func TestOpcacheReset(t *testing.T) {
 	wg := sync.WaitGroup{}
 	numRequests := 500 // increase if test is flaky to get consistent failures
 	wg.Add(numRequests)
+	// pseudo random sleep simulator
 	for i := 0; i < numRequests; i++ {
-		// introduce some random delay
-		if rand.IntN(10) > 8 {
+		if i%10 == 0 {
 			time.Sleep(time.Millisecond * 10)
 		}
 
@@ -1785,8 +1784,8 @@ func TestOpcacheReset(t *testing.T) {
 				return
 			}
 
-			// otherwise call sleep.php with random sleep and work values
-			sleep := i % 100
+			// otherwise call sleep.php with sleep and work values
+			sleep := i % 50
 			work := i % 100
 			tester.AssertGetResponse(
 				fmt.Sprintf("http://localhost:%s/sleep.php?sleep=%d&work=%d", testPort, sleep, work),

frankenphp.c

@@ -239,11 +239,16 @@ static void frankenphp_update_request_context() {
 #if PHP_VERSION_ID < 80300
   zend_function *func = zend_hash_str_find_ptr(
       CG(function_table), "opcache_reset", sizeof("opcache_reset") - 1);
-  if (func != NULL && func->type == ZEND_INTERNAL_FUNCTION) {
+  if (func != NULL && func->type == ZEND_INTERNAL_FUNCTION &&
+      ((zend_internal_function *)func)->handler !=
+          ZEND_FN(frankenphp_opcache_reset)) {
     pthread_mutex_lock(&opcache_reset_mutex_php_82);
-    orig_opcache_reset = ((zend_internal_function *)func)->handler;
-    ((zend_internal_function *)func)->handler =
-        ZEND_FN(frankenphp_opcache_reset);
+    if (((zend_internal_function *)func)->handler !=
+        ZEND_FN(frankenphp_opcache_reset)) {
+      orig_opcache_reset = ((zend_internal_function *)func)->handler;
+      ((zend_internal_function *)func)->handler =
+          ZEND_FN(frankenphp_opcache_reset);
+    }
     pthread_mutex_unlock(&opcache_reset_mutex_php_82);
   }
 #endif

frankenphp.go

@@ -765,21 +765,29 @@ func go_is_context_done(threadIndex C.uintptr_t) C.bool {
 func go_schedule_opcache_reset(threadIndex C.uintptr_t) {
 	if threadsAreRestarting.CompareAndSwap(false, true) {
 		go func() {
+			defer threadsAreRestarting.Store(false)
 			restartThreadsAndOpcacheReset(true)
-			threadsAreRestarting.Store(false)
 		}()
 	}
 }
 
-// restart all threads for a safe opcache_reset
+// opcacheResetOnce ensures only one thread per restart generation calls
+// the actual opcache_reset; concurrent calls into opcache can corrupt SHM
+var opcacheResetOnce atomic.Pointer[sync.Once]
+
+func init() {
+	opcacheResetOnce.Store(&sync.Once{})
+}
+
+// restart all threads for an opcache_reset
 func restartThreadsAndOpcacheReset(withRegularThreads bool) {
 	// disallow scaling threads while restarting workers
 	scalingMu.Lock()
 	defer scalingMu.Unlock()
 
 	threadsToRestart := drainThreads(withRegularThreads)
 
-	opcacheResetOnce = sync.Once{}
+	opcacheResetOnce.Store(&sync.Once{})
 	opcacheResetWg := sync.WaitGroup{}
 	for _, thread := range threadsToRestart {
 		thread.state.Set(state.OpcacheResetting)
@@ -877,7 +885,7 @@ func drainThreads(withRegularThreads bool) []*phpThread {
 }
 
 func scheduleOpcacheReset(thread *phpThread) {
-	opcacheResetOnce.Do(func() {
+	opcacheResetOnce.Load().Do(func() {
 		C.frankenphp_reset_opcache()
 	})
 }