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.

Author: nacholibre

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)
 

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;
 }
 

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;