From a32daf05789149b81c9fb3c962ed3f80f690250b Mon Sep 17 00:00:00 2001 From: David Stone Date: Thu, 16 Apr 2026 18:49:29 -0600 Subject: [PATCH 1/2] fix: drain PHP threads during opcache restart to prevent heap corruption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PHP's opcache uses fcntl() file locks for activity tracking, which are per-process, not per-thread. In FrankenPHP's threaded model, when any single thread releases its lock (request shutdown), it releases for ALL threads — making accel_is_inactive() return true while other threads are still mid-request reading from shared memory. This allows opcache to reset interned strings and hash tables via memset while other threads dereference pointers into that memory, causing zend_mm_heap corrupted crashes every ~30 minutes under normal WordPress traffic. The fix uses a pthread read-write lock around the request lifecycle: - Normal requests: read lock (concurrent, zero contention) - When opcache schedules a restart (OOM, hash overflow, opcache_reset): the zend_accel_schedule_restart_hook sets a flag - The next php_request_startup() sees the flag and takes a write lock, which blocks until all current requests finish their shutdown. Inside that exclusive startup, opcache's accel_is_inactive() correctly sees no active threads and performs the reset safely. - After startup completes, the lock is released and all threads resume. Crash backtrace that prompted this fix: #0 accel_interned_strings_restore_state() — memset zeroing shared memory (concurrent with) #1 zend_hash_del → zend_string_release → _efree → zend_mm_panic Fixes: https://github.com/php/frankenphp/issues/1737 Related: https://github.com/php/php-src/issues/14471 Related: https://github.com/php/frankenphp/issues/2265 --- frankenphp.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/frankenphp.c b/frankenphp.c index 7f396989a0..32a3b9b9bb 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -1,5 +1,6 @@ #include "frankenphp.h" #include +#include #include #include #include @@ -83,6 +84,38 @@ bool original_user_abort_setting = 0; frankenphp_interned_strings_t frankenphp_strings = {0}; HashTable *main_thread_env = NULL; +/* Opcache restart safety — prevents zend_mm_heap corrupted crashes under ZTS. + * + * PHP's opcache activity tracking uses fcntl() file locks, which are + * per-process, not per-thread. In FrankenPHP's threaded model, one thread + * releasing the lock releases it for ALL threads, allowing opcache to reset + * shared memory (interned strings, hash table) while other threads are still + * reading from it. See: https://github.com/php/frankenphp/issues/1737 + * + * Fix: use a pthread read-write lock around the request lifecycle. + * - Normal operation: all threads hold a read lock from before + * php_request_startup() through php_request_shutdown() (concurrent). + * - When the opcache restart hook fires (from a thread mid-request that + * triggered an OOM/hash overflow), it sets a flag. After the triggering + * thread finishes its request and releases its read lock, the NEXT + * php_request_startup() call from ANY thread will acquire a write lock + * instead — blocking until all other threads' current requests complete. + * Inside that exclusive startup, opcache performs the actual reset + * (accel_is_inactive() returns true because no other threads hold the + * fcntl read lock). After startup completes, the write lock is downgraded + * to a read lock and all other threads resume. + */ +static pthread_rwlock_t frankenphp_opcache_rwlock = PTHREAD_RWLOCK_INITIALIZER; +static volatile int frankenphp_opcache_restart_pending = 0; + +static void frankenphp_opcache_restart_hook(int reason) { + (void)reason; + /* Signal that the next php_request_startup() should acquire exclusive + * access. The calling thread is still mid-request (holding a read lock), + * so the exclusive lock will only be acquired after it finishes. */ + __atomic_store_n(&frankenphp_opcache_restart_pending, 1, __ATOMIC_RELEASE); +} + __thread uintptr_t thread_index; __thread bool is_worker_thread = false; __thread HashTable *sandboxed_env = NULL; @@ -1071,7 +1104,22 @@ static void *php_thread(void *arg) { frankenphp_update_request_context(); + /* Opcache restart safety: if a restart was scheduled, ONE thread must + * execute php_request_startup() exclusively (write lock) so the + * opcache reset proceeds while no other thread touches shared memory. + * All other threads take a read lock (concurrent). */ + if (__atomic_load_n(&frankenphp_opcache_restart_pending, __ATOMIC_ACQUIRE)) { + /* Try to become the exclusive restart thread. If another thread + * already took the write lock, this blocks until it finishes. */ + pthread_rwlock_wrlock(&frankenphp_opcache_rwlock); + /* Clear the flag — the reset will happen inside our startup. */ + __atomic_store_n(&frankenphp_opcache_restart_pending, 0, __ATOMIC_RELEASE); + } else { + pthread_rwlock_rdlock(&frankenphp_opcache_rwlock); + } + if (UNEXPECTED(php_request_startup() == FAILURE)) { + pthread_rwlock_unlock(&frankenphp_opcache_rwlock); /* Request startup failed, bail out to zend_catch */ frankenphp_log_message("Request startup failed, thread is unhealthy", LOG_ERR); @@ -1097,6 +1145,7 @@ static void *php_thread(void *arg) { /* shutdown the request, potential bailout to zend_catch */ php_request_shutdown((void *)0); + pthread_rwlock_unlock(&frankenphp_opcache_rwlock); frankenphp_free_request_context(); go_frankenphp_after_script_execution(thread_index, EG(exit_status)); } @@ -1112,6 +1161,7 @@ static void *php_thread(void *arg) { zend_catch {} zend_end_try(); } + pthread_rwlock_unlock(&frankenphp_opcache_rwlock); /* Log the last error message, it must be cleared to prevent a crash when * freeing execution globals */ @@ -1231,6 +1281,17 @@ static void *php_main(void *arg) { frankenphp_sapi_module.startup(&frankenphp_sapi_module); +#ifdef ZTS + /* Register the opcache restart drain hook. + * zend_accel_schedule_restart_hook is a global function pointer declared in + * Zend/zend.h that opcache calls when a restart is scheduled. By hooking it, + * we drain all PHP threads to a safe inter-request boundary before the + * destructive shared memory reset (interned strings, hash table) proceeds. + * This prevents the zend_mm_heap corrupted crashes caused by fcntl file locks + * being per-process rather than per-thread. */ + zend_accel_schedule_restart_hook = frankenphp_opcache_restart_hook; +#endif + /* check if a default filter is set in php.ini and only filter if * it is, this is deprecated and will be removed in PHP 9 */ char *default_filter; From 749e72d9cdde2ce9eacfca80500d0810681880b9 Mon Sep 17 00:00:00 2001 From: David Stone Date: Thu, 16 Apr 2026 19:25:16 -0600 Subject: [PATCH 2/2] fix: guard opcache restart hook for PHP 8.4+ and fix clang-format MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - zend_accel_schedule_restart_hook was added in PHP 8.4; wrap hook function and registration in #if PHP_VERSION_ID >= 80400 to fix build on PHP 8.2 and 8.3. - Break long __atomic_* lines to satisfy clang-format column limit. - The rwlock around the request lifecycle is unconditional (all PHP versions) — it's harmless on 8.2/8.3 (read lock is a no-contention CAS) and the hook simply doesn't fire without the restart hook. --- frankenphp.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index 32a3b9b9bb..d54e174c5d 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -108,13 +108,17 @@ HashTable *main_thread_env = NULL; static pthread_rwlock_t frankenphp_opcache_rwlock = PTHREAD_RWLOCK_INITIALIZER; static volatile int frankenphp_opcache_restart_pending = 0; +#if defined(ZTS) && PHP_VERSION_ID >= 80400 static void frankenphp_opcache_restart_hook(int reason) { (void)reason; /* Signal that the next php_request_startup() should acquire exclusive - * access. The calling thread is still mid-request (holding a read lock), - * so the exclusive lock will only be acquired after it finishes. */ - __atomic_store_n(&frankenphp_opcache_restart_pending, 1, __ATOMIC_RELEASE); + * access. The calling thread is still mid-request (holding a read + * lock), so the exclusive lock will only be acquired after it + * finishes. */ + __atomic_store_n(&frankenphp_opcache_restart_pending, 1, + __ATOMIC_RELEASE); } +#endif __thread uintptr_t thread_index; __thread bool is_worker_thread = false; @@ -1104,16 +1108,18 @@ static void *php_thread(void *arg) { frankenphp_update_request_context(); - /* Opcache restart safety: if a restart was scheduled, ONE thread must - * execute php_request_startup() exclusively (write lock) so the - * opcache reset proceeds while no other thread touches shared memory. - * All other threads take a read lock (concurrent). */ - if (__atomic_load_n(&frankenphp_opcache_restart_pending, __ATOMIC_ACQUIRE)) { - /* Try to become the exclusive restart thread. If another thread - * already took the write lock, this blocks until it finishes. */ + /* Opcache restart safety: if a restart was scheduled, ONE thread + * must execute php_request_startup() exclusively (write lock) so + * the opcache reset proceeds while no other thread touches shared + * memory. All other threads take a read lock (concurrent). */ + if (__atomic_load_n(&frankenphp_opcache_restart_pending, + __ATOMIC_ACQUIRE)) { + /* Become the exclusive restart thread. If another thread already + * took the write lock, this blocks until it finishes. */ pthread_rwlock_wrlock(&frankenphp_opcache_rwlock); - /* Clear the flag — the reset will happen inside our startup. */ - __atomic_store_n(&frankenphp_opcache_restart_pending, 0, __ATOMIC_RELEASE); + /* Clear the flag — reset will happen inside our startup. */ + __atomic_store_n(&frankenphp_opcache_restart_pending, 0, + __ATOMIC_RELEASE); } else { pthread_rwlock_rdlock(&frankenphp_opcache_rwlock); } @@ -1281,13 +1287,13 @@ static void *php_main(void *arg) { frankenphp_sapi_module.startup(&frankenphp_sapi_module); -#ifdef ZTS +#if defined(ZTS) && PHP_VERSION_ID >= 80400 /* Register the opcache restart drain hook. * zend_accel_schedule_restart_hook is a global function pointer declared in - * Zend/zend.h that opcache calls when a restart is scheduled. By hooking it, - * we drain all PHP threads to a safe inter-request boundary before the - * destructive shared memory reset (interned strings, hash table) proceeds. - * This prevents the zend_mm_heap corrupted crashes caused by fcntl file locks + * Zend/zend.h (PHP 8.4+) that opcache calls when a restart is scheduled. + * By hooking it, we drain all PHP threads to a safe inter-request boundary + * before the destructive shared memory reset proceeds. + * This prevents zend_mm_heap corrupted crashes caused by fcntl file locks * being per-process rather than per-thread. */ zend_accel_schedule_restart_hook = frankenphp_opcache_restart_hook; #endif