From 430f1be63f4aa262f1ca4abd5e7f16c16b2e768a Mon Sep 17 00:00:00 2001 From: Yoan Arnaudov <1738437+nacholibre@users.noreply.github.com> Date: Thu, 11 Jun 2026 22:40:04 +0300 Subject: [PATCH] Fix GH-8739: OPcache restart corrupts SHM in threaded SAPIs (ZTS) Before executing a scheduled restart (OOM, hash overflow or opcache_reset()), accel_is_inactive() checks that no request is still reading shared memory. On POSIX this check is implemented with fcntl() record locks: readers take an F_RDLCK in accel_activate_add() and the restarting thread probes for it with F_GETLK. POSIX record locks are per-process: a process never conflicts with its own locks. In threaded SAPIs (FrankenPHP, mod_php with a threaded MPM), all in-flight requests are threads of one process, so F_GETLK never reports a conflict and accel_is_inactive() always returns true. The restart then runs zend_accel_hash_clean() and accel_interned_strings_restore_state() while other threads execute op_arrays and hold interned string pointers in the wiped memory, corrupting the heap ("zend_mm_heap corrupted", SIGSEGV/SIGABRT). Fix it the way ZEND_WIN32 already solved the same problem for threaded SAPIs on Windows: track in-flight requests with an atomic counter in SHM. Requests register in RINIT (only while the accelerator is enabled, so registrations stop as soon as a restart is pending and the counter drains within the duration of the longest in-flight request) and deregister in accel_post_deactivate(). accel_is_inactive() refuses to restart while the counter is non-zero. The counter is only compiled in for ZTS non-Windows builds; NTS process-based SAPIs (php-fpm, mod_php prefork) keep using fcntl() locks, whose kernel-side cleanup on process death they rely on. Verified on a production Symfony workload under FrankenPHP (256 threads, ~120 concurrent requests): repeated opcache_reset() calls and wasted-memory-exhaustion restarts, both previously crashing within seconds, now complete safely after draining readers. --- NEWS | 3 ++ ext/opcache/ZendAccelerator.c | 58 +++++++++++++++++++++++++++++++++++ ext/opcache/ZendAccelerator.h | 7 +++++ 3 files changed, 68 insertions(+) diff --git a/NEWS b/NEWS index 2698420c7a5a..16737a3849d7 100644 --- a/NEWS +++ b/NEWS @@ -62,6 +62,9 @@ PHP NEWS - Opcache: . Fixed tracing JIT crash when a VM interrupt is handled during an observed user function call. (Levi Morrison) + . Fixed bug GH-8739 (OPcache restart corrupts shared memory under threaded + SAPIs: fcntl()-based reader tracking cannot see threads of the same + process). (Yoan Arnaudov) . Fixed bug GH-22004 (Assertion failure at ext/opcache/jit/zend_jit_trace.c). (Arnaud) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index dca5f607ad39..453ced9f6d5a 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -402,6 +402,38 @@ static inline void accel_unlock_all(void) #endif } +#if defined(ZTS) && !defined(ZEND_WIN32) +/* In threaded SAPIs (FrankenPHP, mod_php with a threaded MPM, ...) requests + * run as threads of one process. The fcntl() record locks used by + * accel_activate_add()/accel_is_inactive() are per-process: a process never + * conflicts with its own locks, so in-flight readers running as threads are + * invisible to accel_is_inactive(). It then wrongly reports the cache as + * inactive and a scheduled restart wipes SHM (hash table + interned strings) + * under running requests, corrupting the heap (GH-8739, GH-14471, GH-18517). + * + * Mirror the approach ZEND_WIN32 already uses for the same reason (threaded + * SAPIs): track in-flight requests with an atomic counter in SHM and have + * accel_is_inactive() refuse to restart while it is non-zero. Registration + * is gated on ZCG(accelerator_enabled), so once a restart is pending (the + * accelerator is disabled) new requests no longer register and the counter + * drains within the duration of the longest in-flight request. */ +static zend_always_inline void accel_ts_request_enter(void) +{ + if (!ZCG(ts_reader_registered)) { + __atomic_add_fetch(&ZCSG(ts_active_requests), 1, __ATOMIC_SEQ_CST); + ZCG(ts_reader_registered) = true; + } +} + +static zend_always_inline void accel_ts_request_leave(void) +{ + if (ZCG(ts_reader_registered)) { + __atomic_sub_fetch(&ZCSG(ts_active_requests), 1, __ATOMIC_SEQ_CST); + ZCG(ts_reader_registered) = false; + } +} +#endif + /* Interned strings support */ /* O+ disables creation of interned strings by regular PHP compiler, instead, @@ -894,6 +926,13 @@ static inline void kill_all_lockers(struct flock *mem_usage_check) static inline bool accel_is_inactive(void) { +#if defined(ZTS) && !defined(ZEND_WIN32) + /* Threads of this process may still be inside requests registered as + * SHM readers; the fcntl() probe below cannot detect them. */ + if (__atomic_load_n(&ZCSG(ts_active_requests), __ATOMIC_SEQ_CST) != 0) { + return false; + } +#endif #ifdef ZEND_WIN32 /* on Windows, we don't need kill_all_lockers() because SAPIs that work on Windows don't manage child processes (and we @@ -2742,6 +2781,17 @@ ZEND_RINIT_FUNCTION(zend_accelerator) ZCG(accelerator_enabled) = ZCSG(accelerator_enabled); +#if defined(ZTS) && !defined(ZEND_WIN32) + if (ZCG(accelerator_enabled)) { + accel_ts_request_enter(); + if (UNEXPECTED(ZCSG(restart_in_progress))) { + /* lost the race against a restart that passed accel_is_inactive() + * before our registration became visible; run uncached */ + ZCG(accelerator_enabled) = false; + } + } +#endif + SHM_PROTECT(); HANDLE_UNBLOCK_INTERRUPTIONS(); @@ -2793,6 +2843,14 @@ zend_result accel_post_deactivate(void) accel_unlock_all(); ZCG(counted) = false; +#if defined(ZTS) && !defined(ZEND_WIN32) + if (!file_cache_only && accel_shared_globals) { + SHM_UNPROTECT(); + accel_ts_request_leave(); + SHM_PROTECT(); + } +#endif + return SUCCESS; } diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index 486074ef0012..4031e44d14f4 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -197,6 +197,7 @@ typedef struct _zend_accel_directives { typedef struct _zend_accel_globals { bool counted; /* the process uses shared memory */ + bool ts_reader_registered; /* this request is counted in ZCSG(ts_active_requests) (ZTS only) */ bool enabled; bool locked; /* thread obtained exclusive lock */ bool accelerator_enabled; /* accelerator enabled for current request */ @@ -267,6 +268,12 @@ typedef struct _zend_accel_shared_globals { #ifdef ZEND_WIN32 LONGLONG mem_usage; LONGLONG restart_in; +#elif defined(ZTS) + /* In-flight requests registered as SHM readers. POSIX fcntl() record + * locks cannot track readers that are threads of one process (a process + * never conflicts with its own locks), so threaded SAPIs need an + * explicit counter — mirroring what ZEND_WIN32 does with mem_usage. */ + uint32_t ts_active_requests; #endif bool restart_in_progress; bool jit_counters_stopped;