Skip to content

Commit 032697d

Browse files
committed
Fix opcache_reset()/opcache_invalidate() races using epoch-based reclamation
opcache_reset() and opcache_invalidate() destroy shared memory (hash table memset, interned strings restore, SHM watermark reset) while concurrent reader threads/processes still hold pointers into that memory. This causes zend_mm_heap corrupted crashes under concurrent load in both ZTS (FrankenPHP, parallel) and FPM configurations. This patch introduces epoch-based reclamation (EBR), a proven lock-free pattern used in the Linux kernel (RCU), FreeBSD, and Crossbeam (Rust): - Readers publish their epoch on request start (one atomic store) and clear it on request end (one atomic store). No locks acquired. - Writers (opcache_reset) increment the global epoch and defer the actual cleanup until all pre-epoch readers have completed. - The deferred reset completes at the next request boundary after all old-epoch readers have left. This satisfies the constraint from Dmitry Stogov's review of PR #14803: lock-free cache lookups are preserved. The per-request cost is two atomic stores to cache-line-padded slots (no false sharing). The legacy immediate-reset path is retained as a fast path when no readers are active (accel_is_inactive() returns true). Fixes GH-8739 Fixes GH-14471 Fixes GH-18517
1 parent cef6fbe commit 032697d

File tree

6 files changed

+351
-1
lines changed

6 files changed

+351
-1
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ PHP NEWS
55
- Curl:
66
. Add support for brotli and zstd on Windows. (Shivam Mathur)
77

8+
- OPcache:
9+
. Fixed bug GH-8739 (opcache_reset()/opcache_invalidate() race causes
10+
zend_mm_heap corrupted under concurrent load in ZTS and FPM). (superdav42)
11+
812
15 Jan 2026, PHP 8.3.30
913

1014
- Core:

ext/opcache/ZendAccelerator.c

Lines changed: 198 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,8 @@ static zend_result (*orig_post_startup_cb)(void);
132132

133133
static zend_result accel_post_startup(void);
134134
static zend_result accel_finish_startup(void);
135+
static void zend_reset_cache_vars(void);
136+
static void accel_interned_strings_restore_state(void);
135137

136138
static void preload_shutdown(void);
137139
static void preload_activate(void);
@@ -398,6 +400,147 @@ static inline void accel_unlock_all(void)
398400
#endif
399401
}
400402

403+
/* ====================================================================
404+
* Epoch-based reclamation (EBR) for safe opcache_reset()/invalidate()
405+
*
406+
* Solves: php-src#8739, #14471, #18517, frankenphp#1737, #2265, #2170
407+
*
408+
* Design:
409+
* - Readers (per-request) publish their epoch on enter, clear on leave.
410+
* Cost: two atomic stores per request. No locks.
411+
* - Writers (opcache_reset) increment the global epoch and defer the
412+
* actual cleanup until all pre-epoch readers have completed.
413+
* - This guarantees that no reader holds a pointer to shared memory
414+
* that is about to be reclaimed.
415+
* ==================================================================== */
416+
417+
void accel_epoch_init(void)
418+
{
419+
int i;
420+
421+
ZCSG(current_epoch) = 1;
422+
ZCSG(drain_epoch) = 0;
423+
ZCSG(reset_deferred) = false;
424+
ZCSG(epoch_slot_next) = 0;
425+
426+
for (i = 0; i < ACCEL_EPOCH_MAX_SLOTS; i++) {
427+
ZCSG(epoch_slots)[i].epoch = ACCEL_EPOCH_INACTIVE;
428+
}
429+
}
430+
431+
static int accel_epoch_alloc_slot(void)
432+
{
433+
int slot;
434+
435+
#if defined(ZEND_WIN32)
436+
slot = (int)InterlockedIncrement((volatile LONG *)&ZCSG(epoch_slot_next)) - 1;
437+
#elif defined(__GNUC__)
438+
slot = __atomic_fetch_add(&ZCSG(epoch_slot_next), 1, __ATOMIC_SEQ_CST);
439+
#else
440+
slot = ZCSG(epoch_slot_next)++;
441+
#endif
442+
443+
if (slot >= ACCEL_EPOCH_MAX_SLOTS) {
444+
return ACCEL_EPOCH_NO_SLOT;
445+
}
446+
return slot;
447+
}
448+
449+
void accel_epoch_enter(void)
450+
{
451+
int slot = ZCG(epoch_slot);
452+
453+
if (UNEXPECTED(slot == ACCEL_EPOCH_NO_SLOT)) {
454+
slot = accel_epoch_alloc_slot();
455+
ZCG(epoch_slot) = slot;
456+
}
457+
458+
if (EXPECTED(slot >= 0 && slot < ACCEL_EPOCH_MAX_SLOTS)) {
459+
uint64_t epoch = ACCEL_ATOMIC_LOAD_64(&ZCSG(current_epoch));
460+
ZCG(local_epoch) = epoch;
461+
ACCEL_ATOMIC_STORE_64(&ZCSG(epoch_slots)[slot].epoch, epoch);
462+
}
463+
}
464+
465+
void accel_epoch_leave(void)
466+
{
467+
int slot = ZCG(epoch_slot);
468+
469+
if (EXPECTED(slot >= 0 && slot < ACCEL_EPOCH_MAX_SLOTS)) {
470+
ACCEL_ATOMIC_STORE_64(&ZCSG(epoch_slots)[slot].epoch, ACCEL_EPOCH_INACTIVE);
471+
}
472+
}
473+
474+
static uint64_t accel_min_active_epoch(void)
475+
{
476+
uint64_t min_epoch = ACCEL_EPOCH_INACTIVE;
477+
int i;
478+
479+
for (i = 0; i < ACCEL_EPOCH_MAX_SLOTS; i++) {
480+
uint64_t e = ACCEL_ATOMIC_LOAD_64(&ZCSG(epoch_slots)[i].epoch);
481+
if (e < min_epoch) {
482+
min_epoch = e;
483+
}
484+
}
485+
return min_epoch;
486+
}
487+
488+
bool accel_deferred_reset_pending(void)
489+
{
490+
return ZCSG(reset_deferred);
491+
}
492+
493+
void accel_try_complete_deferred_reset(void)
494+
{
495+
uint64_t drain_epoch;
496+
uint64_t min_epoch;
497+
498+
if (!ZCSG(reset_deferred)) {
499+
return;
500+
}
501+
502+
drain_epoch = ACCEL_ATOMIC_LOAD_64(&ZCSG(drain_epoch));
503+
min_epoch = accel_min_active_epoch();
504+
505+
if (min_epoch > drain_epoch) {
506+
zend_shared_alloc_lock();
507+
508+
if (ZCSG(reset_deferred)) {
509+
zend_accel_error(ACCEL_LOG_DEBUG, "Completing deferred opcache reset (drain_epoch=%" PRIu64 ", min_active=%" PRIu64 ")",
510+
drain_epoch, min_epoch);
511+
512+
zend_map_ptr_reset();
513+
zend_reset_cache_vars();
514+
zend_accel_hash_clean(&ZCSG(hash));
515+
516+
if (ZCG(accel_directives).interned_strings_buffer) {
517+
accel_interned_strings_restore_state();
518+
}
519+
520+
zend_shared_alloc_restore_state();
521+
522+
if (ZCSG(preload_script)) {
523+
preload_restart();
524+
}
525+
526+
#ifdef HAVE_JIT
527+
zend_jit_restart();
528+
#endif
529+
530+
ZCSG(accelerator_enabled) = ZCSG(cache_status_before_restart);
531+
if (ZCSG(last_restart_time) < ZCG(request_time)) {
532+
ZCSG(last_restart_time) = ZCG(request_time);
533+
} else {
534+
ZCSG(last_restart_time)++;
535+
}
536+
537+
ZCSG(reset_deferred) = false;
538+
}
539+
540+
zend_shared_alloc_unlock();
541+
}
542+
}
543+
401544
/* Interned strings support */
402545

403546
/* O+ disables creation of interned strings by regular PHP compiler, instead,
@@ -2671,11 +2814,24 @@ zend_result accel_activate(INIT_FUNC_ARGS)
26712814
ZCG(counted) = false;
26722815
}
26732816

2817+
/* Enter the current epoch BEFORE checking for pending restart.
2818+
* This ensures that if a reset is initiated after this point,
2819+
* the deferred-reset logic will wait for us to leave. */
2820+
accel_epoch_enter();
2821+
2822+
/* Check if a deferred reset from a previous opcache_reset() can
2823+
* now be completed (all pre-epoch readers have finished). */
2824+
if (ZCSG(reset_deferred)) {
2825+
accel_try_complete_deferred_reset();
2826+
}
2827+
26742828
if (ZCSG(restart_pending)) {
26752829
zend_shared_alloc_lock();
26762830
if (ZCSG(restart_pending)) { /* check again, to ensure that the cache wasn't already cleaned by another process */
26772831
if (accel_is_inactive()) {
2678-
zend_accel_error(ACCEL_LOG_DEBUG, "Restarting!");
2832+
/* All processes/threads are inactive (legacy flock check).
2833+
* We can do the cleanup immediately. */
2834+
zend_accel_error(ACCEL_LOG_DEBUG, "Restarting! (immediate — no active readers)");
26792835
ZCSG(restart_pending) = false;
26802836
switch ZCSG(restart_reason) {
26812837
case ACCEL_RESTART_OOM:
@@ -2714,6 +2870,31 @@ zend_result accel_activate(INIT_FUNC_ARGS)
27142870
ZCSG(last_restart_time)++;
27152871
}
27162872
accel_restart_leave();
2873+
} else if (!ZCSG(reset_deferred)) {
2874+
/* Readers are still active. Defer the actual cleanup to a
2875+
* future request end, after all current readers have left.
2876+
* This is the core of the epoch-based reclamation approach. */
2877+
zend_accel_error(ACCEL_LOG_DEBUG, "Deferring opcache restart (active readers detected)");
2878+
ZCSG(restart_pending) = false;
2879+
2880+
switch ZCSG(restart_reason) {
2881+
case ACCEL_RESTART_OOM:
2882+
ZCSG(oom_restarts)++;
2883+
break;
2884+
case ACCEL_RESTART_HASH:
2885+
ZCSG(hash_restarts)++;
2886+
break;
2887+
case ACCEL_RESTART_USER:
2888+
ZCSG(manual_restarts)++;
2889+
break;
2890+
}
2891+
2892+
ACCEL_ATOMIC_STORE_64(&ZCSG(drain_epoch), ACCEL_ATOMIC_LOAD_64(&ZCSG(current_epoch)));
2893+
ACCEL_ATOMIC_INC_64(&ZCSG(current_epoch));
2894+
2895+
ZCSG(cache_status_before_restart) = ZCSG(accelerator_enabled);
2896+
ZCSG(accelerator_enabled) = false;
2897+
ZCSG(reset_deferred) = true;
27172898
}
27182899
}
27192900
zend_shared_alloc_unlock();
@@ -2768,6 +2949,18 @@ zend_result accel_post_deactivate(void)
27682949
return SUCCESS;
27692950
}
27702951

2952+
/* Leave the epoch — this thread/process no longer holds references
2953+
* to shared OPcache data. */
2954+
accel_epoch_leave();
2955+
2956+
/* If a deferred reset is pending, check if we were the last reader
2957+
* from the old epoch. If so, complete the reset now. */
2958+
if (!file_cache_only && accel_shared_globals && ZCSG(reset_deferred)) {
2959+
SHM_UNPROTECT();
2960+
accel_try_complete_deferred_reset();
2961+
SHM_PROTECT();
2962+
}
2963+
27712964
zend_shared_alloc_safe_unlock(); /* be sure we didn't leave cache locked */
27722965
accel_unlock_all();
27732966
ZCG(counted) = false;
@@ -2911,6 +3104,9 @@ static zend_result zend_accel_init_shm(void)
29113104

29123105
zend_reset_cache_vars();
29133106

3107+
/* Initialize epoch-based reclamation tracking */
3108+
accel_epoch_init();
3109+
29143110
ZCSG(oom_restarts) = 0;
29153111
ZCSG(hash_restarts) = 0;
29163112
ZCSG(manual_restarts) = 0;
@@ -2937,6 +3133,7 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
29373133
memset(accel_globals, 0, sizeof(zend_accel_globals));
29383134
accel_globals->key = zend_string_alloc(ZCG_KEY_LEN, true);
29393135
GC_MAKE_PERSISTENT_LOCAL(accel_globals->key);
3136+
accel_globals->epoch_slot = ACCEL_EPOCH_NO_SLOT;
29403137
}
29413138

29423139
static void accel_globals_dtor(zend_accel_globals *accel_globals)

ext/opcache/ZendAccelerator.h

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,52 @@
5151
#include "zend_extensions.h"
5252
#include "zend_compile.h"
5353

54+
#include <stdint.h>
55+
5456
#include "Optimizer/zend_optimizer.h"
5557
#include "zend_accelerator_hash.h"
5658
#include "zend_accelerator_debug.h"
5759

60+
/* Maximum number of concurrent readers tracked for epoch-based reclamation.
61+
* Each slot is padded to a cache line to avoid false sharing.
62+
* This limits the number of concurrent threads (ZTS) or processes (FPM)
63+
* that can be tracked. Excess readers degrade to the legacy flock-based path. */
64+
#define ACCEL_EPOCH_MAX_SLOTS 256
65+
66+
/* Sentinel value meaning "this slot is not in an active request" */
67+
#define ACCEL_EPOCH_INACTIVE UINT64_MAX
68+
69+
/* Sentinel value meaning "no slot has been assigned to this thread/process" */
70+
#define ACCEL_EPOCH_NO_SLOT (-1)
71+
72+
/*
73+
* Portable 64-bit atomic operations for epoch tracking.
74+
* These use compiler builtins matching the patterns in Zend/zend_atomic.h,
75+
* extended to uint64_t which zend_atomic.h does not cover.
76+
*/
77+
#if defined(ZEND_WIN32)
78+
# define ACCEL_ATOMIC_LOAD_64(ptr) InterlockedCompareExchange64((volatile LONG64 *)(ptr), 0, 0)
79+
# define ACCEL_ATOMIC_STORE_64(ptr, val) InterlockedExchange64((volatile LONG64 *)(ptr), (LONG64)(val))
80+
# define ACCEL_ATOMIC_INC_64(ptr) InterlockedIncrement64((volatile LONG64 *)(ptr))
81+
#elif defined(__clang__) && __has_feature(c_atomic)
82+
# define ACCEL_ATOMIC_LOAD_64(ptr) __c11_atomic_load((_Atomic(uint64_t) *)(ptr), __ATOMIC_ACQUIRE)
83+
# define ACCEL_ATOMIC_STORE_64(ptr, val) __c11_atomic_store((_Atomic(uint64_t) *)(ptr), (uint64_t)(val), __ATOMIC_RELEASE)
84+
# define ACCEL_ATOMIC_INC_64(ptr) (__c11_atomic_fetch_add((_Atomic(uint64_t) *)(ptr), 1, __ATOMIC_SEQ_CST) + 1)
85+
#elif defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7))
86+
# define ACCEL_ATOMIC_LOAD_64(ptr) __atomic_load_n((volatile uint64_t *)(ptr), __ATOMIC_ACQUIRE)
87+
# define ACCEL_ATOMIC_STORE_64(ptr, val) __atomic_store_n((volatile uint64_t *)(ptr), (uint64_t)(val), __ATOMIC_RELEASE)
88+
# define ACCEL_ATOMIC_INC_64(ptr) __atomic_add_fetch((volatile uint64_t *)(ptr), 1, __ATOMIC_SEQ_CST)
89+
#elif defined(__GNUC__)
90+
# define ACCEL_ATOMIC_LOAD_64(ptr) __sync_fetch_and_or((volatile uint64_t *)(ptr), 0)
91+
# define ACCEL_ATOMIC_STORE_64(ptr, val) do { __sync_synchronize(); *(volatile uint64_t *)(ptr) = (uint64_t)(val); __sync_synchronize(); } while (0)
92+
# define ACCEL_ATOMIC_INC_64(ptr) __sync_add_and_fetch((volatile uint64_t *)(ptr), 1)
93+
#else
94+
/* Fallback: volatile without barriers. Correct on x86 TSO; may need fence on ARM. */
95+
# define ACCEL_ATOMIC_LOAD_64(ptr) (*(volatile uint64_t *)(ptr))
96+
# define ACCEL_ATOMIC_STORE_64(ptr, val) (*(volatile uint64_t *)(ptr) = (uint64_t)(val))
97+
# define ACCEL_ATOMIC_INC_64(ptr) (++(*(volatile uint64_t *)(ptr)))
98+
#endif
99+
58100
#ifndef PHPAPI
59101
# ifdef ZEND_WIN32
60102
# define PHPAPI __declspec(dllimport)
@@ -116,6 +158,14 @@ typedef struct _zend_early_binding {
116158
uint32_t cache_slot;
117159
} zend_early_binding;
118160

161+
/* Per-thread/process epoch slot for safe reclamation.
162+
* Padded to a full cache line (64 bytes) to prevent false sharing
163+
* between concurrently-active reader threads/processes. */
164+
typedef struct _zend_accel_epoch_slot {
165+
volatile uint64_t epoch; /* Current epoch or ACCEL_EPOCH_INACTIVE */
166+
char padding[56]; /* Pad to 64-byte cache line */
167+
} zend_accel_epoch_slot;
168+
119169
typedef struct _zend_persistent_script {
120170
zend_script script;
121171
zend_long compiler_halt_offset; /* position of __HALT_COMPILER or -1 */
@@ -216,6 +266,9 @@ typedef struct _zend_accel_globals {
216266
#ifndef ZEND_WIN32
217267
zend_ulong root_hash;
218268
#endif
269+
/* Epoch-based reclamation: per-thread/process state */
270+
int epoch_slot; /* Index into ZCSG(epoch_slots), or ACCEL_EPOCH_NO_SLOT */
271+
uint64_t local_epoch; /* Snapshot of global epoch at request start */
219272
/* preallocated shared-memory block to save current script */
220273
void *mem;
221274
zend_persistent_script *current_persistent_script;
@@ -273,6 +326,13 @@ typedef struct _zend_accel_shared_globals {
273326
void *jit_traces;
274327
const void **jit_exit_groups;
275328

329+
/* Epoch-based reclamation for safe opcache_reset()/invalidate() (GH#8739) */
330+
volatile uint64_t current_epoch;
331+
volatile uint64_t drain_epoch;
332+
volatile bool reset_deferred;
333+
volatile int32_t epoch_slot_next;
334+
zend_accel_epoch_slot epoch_slots[ACCEL_EPOCH_MAX_SLOTS];
335+
276336
/* Interned Strings Support (must be the last element) */
277337
zend_string_table interned_strings;
278338
} zend_accel_shared_globals;
@@ -319,6 +379,13 @@ void accelerator_shm_read_unlock(void);
319379
zend_string *accel_make_persistent_key(zend_string *path);
320380
zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type);
321381

382+
/* Epoch-based reclamation API */
383+
void accel_epoch_init(void);
384+
void accel_epoch_enter(void);
385+
void accel_epoch_leave(void);
386+
bool accel_deferred_reset_pending(void);
387+
void accel_try_complete_deferred_reset(void);
388+
322389
#define IS_ACCEL_INTERNED(str) \
323390
((char*)(str) >= (char*)ZCSG(interned_strings).start && (char*)(str) < (char*)ZCSG(interned_strings).top)
324391

0 commit comments

Comments
 (0)