Skip to content

Fix GH-8739: opcache_reset()/invalidate() races via epoch-based reclamation #21778

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
superdav42 wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
Open

Fix GH-8739: opcache_reset()/invalidate() races via epoch-based reclamation #21778

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ PHP NEWS
. Fixed bug GH-21593 (Borked function JIT JMPNZ smart branch). (ilutov)
. Fixed bug GH-21460 (COND optimization regression). (Dmitry, Arnaud)
. Fixed faulty returns out of zend_try block in zend_jit_trace(). (ilutov)
. Fixed bug GH-8739 (opcache_reset()/opcache_invalidate() race causes
zend_mm_heap corrupted under concurrent load in ZTS and FPM).
(superdav42)

- OpenSSL:
. Fix a bunch of memory leaks and crashes on edge cases. (ndossche)
Expand Down
222 changes: 222 additions & 0 deletions ext/opcache/ZendAccelerator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ static zend_result (*orig_post_startup_cb)(void);

static zend_result accel_post_startup(void);
static zend_result accel_finish_startup(void);
static void zend_reset_cache_vars(void);
static void accel_interned_strings_restore_state(void);

static void preload_shutdown(void);
static void preload_activate(void);
Expand Down Expand Up @@ -402,6 +404,175 @@ static inline void accel_unlock_all(void)
#endif
}

/* Epoch-based reclamation for safe opcache_reset()/opcache_invalidate()
* under concurrent load (GH-8739). Readers publish their epoch on request
* start and clear it on request end. Writers defer the destructive cleanup
* until every pre-reset reader has finished, so lock-free readers never
* race with hash memset or interned strings restore.
*/

void accel_epoch_init(void)
{
int i;

/* epoch 0 is reserved as the "never deferred" sentinel for drain_epoch */
ZCSG(current_epoch) = 1;
ZCSG(drain_epoch) = 0;
ZCSG(reset_deferred) = false;
ZCSG(epoch_slot_next) = 0;
ZCSG(epoch_overflow_active) = 0;

for (i = 0; i < ACCEL_EPOCH_MAX_SLOTS; i++) {
ZCSG(epoch_slots)[i].epoch = ACCEL_EPOCH_INACTIVE;
}
}

static int32_t accel_epoch_alloc_slot(void)
{
int32_t slot;

#if defined(ZEND_WIN32)
slot = (int32_t)InterlockedIncrement((volatile LONG *)&ZCSG(epoch_slot_next)) - 1;
#elif defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7))
slot = __atomic_fetch_add(&ZCSG(epoch_slot_next), 1, __ATOMIC_SEQ_CST);
#elif defined(__GNUC__)
slot = __sync_fetch_and_add(&ZCSG(epoch_slot_next), 1);
#else
slot = ZCSG(epoch_slot_next)++;
#endif

if (slot < 0 || slot >= ACCEL_EPOCH_MAX_SLOTS) {
return ACCEL_EPOCH_SLOT_OVERFLOW;
}
return slot;
}

void accel_epoch_enter(void)
{
int32_t slot = ZCG(epoch_slot);

if (UNEXPECTED(slot == ACCEL_EPOCH_SLOT_UNASSIGNED)) {
slot = accel_epoch_alloc_slot();
ZCG(epoch_slot) = slot;
}

if (EXPECTED(slot >= 0)) {
uint64_t epoch = ACCEL_ATOMIC_LOAD_64(&ZCSG(current_epoch));
ZCG(local_epoch) = epoch;
ACCEL_ATOMIC_STORE_64(&ZCSG(epoch_slots)[slot].epoch, epoch);
} else {
/* per-reader slots exhausted - fall back to the aggregate counter
* so deferred reclamation still waits for this reader */
ACCEL_ATOMIC_INC_32(&ZCSG(epoch_overflow_active));
ZCG(local_epoch) = 0;
}
}

void accel_epoch_leave(void)
{
int32_t slot = ZCG(epoch_slot);

if (EXPECTED(slot >= 0)) {
ACCEL_ATOMIC_STORE_64(&ZCSG(epoch_slots)[slot].epoch, ACCEL_EPOCH_INACTIVE);
} else if (slot == ACCEL_EPOCH_SLOT_OVERFLOW) {
ACCEL_ATOMIC_DEC_32(&ZCSG(epoch_overflow_active));
}
}

static uint64_t accel_min_active_epoch(void)
{
uint64_t min_epoch = ACCEL_EPOCH_INACTIVE;
int i;

for (i = 0; i < ACCEL_EPOCH_MAX_SLOTS; i++) {
uint64_t e = ACCEL_ATOMIC_LOAD_64(&ZCSG(epoch_slots)[i].epoch);
if (e < min_epoch) {
min_epoch = e;
}
}
return min_epoch;
}

bool accel_deferred_reset_pending(void)
{
return ZCSG(reset_deferred);
}

void accel_try_complete_deferred_reset(void)
{
uint64_t drain_epoch;
uint64_t min_epoch;
uint32_t overflow;

if (!ZCSG(reset_deferred)) {
return;
}

/* re-entering zend_shared_alloc_lock() would hit ZEND_ASSERT(!ZCG(locked)) */
if (ZCG(locked)) {
return;
}

overflow = ACCEL_ATOMIC_LOAD_32(&ZCSG(epoch_overflow_active));
if (overflow > 0) {
return;
}

drain_epoch = ACCEL_ATOMIC_LOAD_64(&ZCSG(drain_epoch));
min_epoch = accel_min_active_epoch();

if (min_epoch <= drain_epoch) {
return;
}

zend_shared_alloc_lock();

if (ZCSG(reset_deferred)) {
/* re-check under lock in case another thread already completed,
* or a reader published drain_epoch after our lock-free check */
min_epoch = accel_min_active_epoch();
if (min_epoch > ACCEL_ATOMIC_LOAD_64(&ZCSG(drain_epoch))) {
zend_accel_error(ACCEL_LOG_DEBUG,
"Completing deferred opcache reset (drain_epoch=%" PRIu64
", min_active_epoch=%" PRIu64 ")",
drain_epoch, min_epoch);

accel_restart_enter();

zend_map_ptr_reset();
zend_reset_cache_vars();
zend_accel_hash_clean(&ZCSG(hash));

if (ZCG(accel_directives).interned_strings_buffer) {
accel_interned_strings_restore_state();
}

zend_shared_alloc_restore_state();

if (ZCSG(preload_script)) {
preload_restart();
}

#ifdef HAVE_JIT
zend_jit_restart();
#endif

ZCSG(accelerator_enabled) = ZCSG(cache_status_before_restart);
if (ZCSG(last_restart_time) < ZCG(request_time)) {
ZCSG(last_restart_time) = ZCG(request_time);
} else {
ZCSG(last_restart_time)++;
}

ZCSG(reset_deferred) = false;

accel_restart_leave();
}
}

zend_shared_alloc_unlock();
}

/* Interned strings support */

/* O+ disables creation of interned strings by regular PHP compiler, instead,
Expand Down Expand Up @@ -2692,6 +2863,14 @@ ZEND_RINIT_FUNCTION(zend_accelerator)
ZCG(counted) = false;
}

/* publish epoch before any shared memory access so a concurrent
* opcache_reset() waits for us (GH-8739) */
accel_epoch_enter();

if (UNEXPECTED(ZCSG(reset_deferred))) {
accel_try_complete_deferred_reset();
}

if (ZCSG(restart_pending)) {
zend_shared_alloc_lock();
if (ZCSG(restart_pending)) { /* check again, to ensure that the cache wasn't already cleaned by another process */
Expand Down Expand Up @@ -2735,6 +2914,35 @@ ZEND_RINIT_FUNCTION(zend_accelerator)
ZCSG(last_restart_time)++;
}
accel_restart_leave();
} else if (!ZCSG(reset_deferred)) {
/* Readers are active — defer the destructive cleanup to
* accel_try_complete_deferred_reset() at the next request
* boundary, after all current readers have drained (GH-8739) */
zend_accel_error(ACCEL_LOG_DEBUG,
"Deferring opcache restart: active readers detected");
ZCSG(restart_pending) = false;

switch ZCSG(restart_reason) {
case ACCEL_RESTART_OOM:
ZCSG(oom_restarts)++;
break;
case ACCEL_RESTART_HASH:
ZCSG(hash_restarts)++;
break;
case ACCEL_RESTART_USER:
ZCSG(manual_restarts)++;
break;
}

/* new readers publish current_epoch+1, which is > drain_epoch,
* so they don't block the drain check */
ACCEL_ATOMIC_STORE_64(&ZCSG(drain_epoch),
ACCEL_ATOMIC_LOAD_64(&ZCSG(current_epoch)));
ACCEL_ATOMIC_INC_64(&ZCSG(current_epoch));

ZCSG(cache_status_before_restart) = ZCSG(accelerator_enabled);
ZCSG(accelerator_enabled) = false;
ZCSG(reset_deferred) = true;
}
}
zend_shared_alloc_unlock();
Expand Down Expand Up @@ -2789,6 +2997,17 @@ zend_result accel_post_deactivate(void)
return SUCCESS;
}

/* leave the epoch before try_complete, so our own slot doesn't block
* the drain check (GH-8739) */
if (!file_cache_only && accel_shared_globals) {
SHM_UNPROTECT();
accel_epoch_leave();
if (UNEXPECTED(ZCSG(reset_deferred))) {
accel_try_complete_deferred_reset();
}
SHM_PROTECT();
}

zend_shared_alloc_safe_unlock(); /* be sure we didn't leave cache locked */
accel_unlock_all();
ZCG(counted) = false;
Expand Down Expand Up @@ -2936,6 +3155,8 @@ static zend_result zend_accel_init_shm(void)

zend_reset_cache_vars();

accel_epoch_init();

ZCSG(oom_restarts) = 0;
ZCSG(hash_restarts) = 0;
ZCSG(manual_restarts) = 0;
Expand All @@ -2962,6 +3183,7 @@ static void accel_globals_ctor(zend_accel_globals *accel_globals)
memset(accel_globals, 0, sizeof(zend_accel_globals));
accel_globals->key = zend_string_alloc(ZCG_KEY_LEN, true);
GC_MAKE_PERSISTENT_LOCAL(accel_globals->key);
accel_globals->epoch_slot = ACCEL_EPOCH_SLOT_UNASSIGNED;
}

static void accel_globals_dtor(zend_accel_globals *accel_globals)
Expand Down
73 changes: 73 additions & 0 deletions ext/opcache/ZendAccelerator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,59 @@
#include "zend_compile.h"
#include "zend_API.h"

#include <stdint.h>

#include "Optimizer/zend_optimizer.h"
#include "zend_accelerator_hash.h"
#include "zend_accelerator_debug.h"

/* Concurrent readers tracked precisely; excess readers fall back to
* ZCSG(epoch_overflow_active) (GH-8739) */
#define ACCEL_EPOCH_MAX_SLOTS 256

#define ACCEL_EPOCH_SLOT_UNASSIGNED (-1) /* slot not yet claimed */
#define ACCEL_EPOCH_SLOT_OVERFLOW (-2) /* claim failed, slots exhausted */
#define ACCEL_EPOCH_INACTIVE UINT64_MAX

/* 64-bit atomics for epoch tracking; zend_atomic.h only covers 32-bit */
#if defined(ZEND_WIN32)
# define ACCEL_ATOMIC_LOAD_64(ptr) ((uint64_t)InterlockedCompareExchange64((volatile LONG64 *)(ptr), 0, 0))
# define ACCEL_ATOMIC_STORE_64(ptr, val) ((void)InterlockedExchange64((volatile LONG64 *)(ptr), (LONG64)(val)))
# define ACCEL_ATOMIC_INC_64(ptr) ((uint64_t)InterlockedIncrement64((volatile LONG64 *)(ptr)))
# define ACCEL_ATOMIC_INC_32(ptr) ((uint32_t)InterlockedIncrement((volatile LONG *)(ptr)))
# define ACCEL_ATOMIC_DEC_32(ptr) ((uint32_t)InterlockedDecrement((volatile LONG *)(ptr)))
# define ACCEL_ATOMIC_LOAD_32(ptr) ((uint32_t)InterlockedCompareExchange((volatile LONG *)(ptr), 0, 0))
#elif defined(__clang__) && defined(__has_feature) && __has_feature(c_atomic)
# define ACCEL_ATOMIC_LOAD_64(ptr) __c11_atomic_load((_Atomic(uint64_t) *)(ptr), __ATOMIC_ACQUIRE)
# define ACCEL_ATOMIC_STORE_64(ptr, val) __c11_atomic_store((_Atomic(uint64_t) *)(ptr), (uint64_t)(val), __ATOMIC_RELEASE)
# define ACCEL_ATOMIC_INC_64(ptr) (__c11_atomic_fetch_add((_Atomic(uint64_t) *)(ptr), 1, __ATOMIC_SEQ_CST) + 1)
# define ACCEL_ATOMIC_INC_32(ptr) (__c11_atomic_fetch_add((_Atomic(uint32_t) *)(ptr), 1, __ATOMIC_SEQ_CST) + 1)
# define ACCEL_ATOMIC_DEC_32(ptr) (__c11_atomic_fetch_sub((_Atomic(uint32_t) *)(ptr), 1, __ATOMIC_SEQ_CST) - 1)
# define ACCEL_ATOMIC_LOAD_32(ptr) __c11_atomic_load((_Atomic(uint32_t) *)(ptr), __ATOMIC_ACQUIRE)
#elif defined(__GNUC__) && (__GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 7))
# define ACCEL_ATOMIC_LOAD_64(ptr) __atomic_load_n((volatile uint64_t *)(ptr), __ATOMIC_ACQUIRE)
# define ACCEL_ATOMIC_STORE_64(ptr, val) __atomic_store_n((volatile uint64_t *)(ptr), (uint64_t)(val), __ATOMIC_RELEASE)
# define ACCEL_ATOMIC_INC_64(ptr) __atomic_add_fetch((volatile uint64_t *)(ptr), 1, __ATOMIC_SEQ_CST)
# define ACCEL_ATOMIC_INC_32(ptr) __atomic_add_fetch((volatile uint32_t *)(ptr), 1, __ATOMIC_SEQ_CST)
# define ACCEL_ATOMIC_DEC_32(ptr) __atomic_sub_fetch((volatile uint32_t *)(ptr), 1, __ATOMIC_SEQ_CST)
# define ACCEL_ATOMIC_LOAD_32(ptr) __atomic_load_n((volatile uint32_t *)(ptr), __ATOMIC_ACQUIRE)
#elif defined(__GNUC__)
# define ACCEL_ATOMIC_LOAD_64(ptr) __sync_fetch_and_or((volatile uint64_t *)(ptr), 0)
# define ACCEL_ATOMIC_STORE_64(ptr, val) do { __sync_synchronize(); *(volatile uint64_t *)(ptr) = (uint64_t)(val); __sync_synchronize(); } while (0)
# define ACCEL_ATOMIC_INC_64(ptr) __sync_add_and_fetch((volatile uint64_t *)(ptr), 1)
# define ACCEL_ATOMIC_INC_32(ptr) __sync_add_and_fetch((volatile uint32_t *)(ptr), 1)
# define ACCEL_ATOMIC_DEC_32(ptr) __sync_sub_and_fetch((volatile uint32_t *)(ptr), 1)
# define ACCEL_ATOMIC_LOAD_32(ptr) __sync_fetch_and_or((volatile uint32_t *)(ptr), 0)
#else
/* fallback: volatile only - correct on x86 TSO, not on weak-memory arches */
# define ACCEL_ATOMIC_LOAD_64(ptr) (*(volatile uint64_t *)(ptr))
# define ACCEL_ATOMIC_STORE_64(ptr, val) (*(volatile uint64_t *)(ptr) = (uint64_t)(val))
# define ACCEL_ATOMIC_INC_64(ptr) (++(*(volatile uint64_t *)(ptr)))
# define ACCEL_ATOMIC_INC_32(ptr) (++(*(volatile uint32_t *)(ptr)))
# define ACCEL_ATOMIC_DEC_32(ptr) (--(*(volatile uint32_t *)(ptr)))
# define ACCEL_ATOMIC_LOAD_32(ptr) (*(volatile uint32_t *)(ptr))
#endif

#ifndef PHPAPI
# ifdef ZEND_WIN32
# define PHPAPI __declspec(dllimport)
Expand Down Expand Up @@ -117,6 +166,12 @@ typedef struct _zend_early_binding {
uint32_t cache_slot;
} zend_early_binding;

/* per-reader epoch publication slot, padded to a cache line against false sharing */
typedef struct _zend_accel_epoch_slot {
volatile uint64_t epoch;
char padding[56];
} zend_accel_epoch_slot;

typedef struct _zend_persistent_script {
zend_script script;
zend_long compiler_halt_offset; /* position of __HALT_COMPILER or -1 */
Expand Down Expand Up @@ -219,6 +274,9 @@ typedef struct _zend_accel_globals {
#endif
void *preloaded_internal_run_time_cache;
size_t preloaded_internal_run_time_cache_size;
/* EBR state: slot index into ZCSG(epoch_slots) or ACCEL_EPOCH_SLOT_* */
int32_t epoch_slot;
uint64_t local_epoch;
/* preallocated shared-memory block to save current script */
void *mem;
zend_persistent_script *current_persistent_script;
Expand Down Expand Up @@ -282,6 +340,14 @@ typedef struct _zend_accel_shared_globals {
void *jit_traces;
const void **jit_exit_groups;

/* Epoch-based reclamation for safe opcache_reset()/invalidate() (GH-8739) */
volatile uint64_t current_epoch; /* bumped on each deferred reset */
volatile uint64_t drain_epoch; /* readers publishing <= this block reclamation */
volatile bool reset_deferred;
volatile int32_t epoch_slot_next; /* monotonic slot allocator */
volatile uint32_t epoch_overflow_active; /* readers that didn't get a slot */
zend_accel_epoch_slot epoch_slots[ACCEL_EPOCH_MAX_SLOTS];

/* Interned Strings Support (must be the last element) */
ZEND_SET_ALIGNED(ZEND_STRING_TABLE_POS_ALIGNMENT, zend_string_table interned_strings);
} zend_accel_shared_globals;
Expand Down Expand Up @@ -328,6 +394,13 @@ void accelerator_shm_read_unlock(void);
zend_string *accel_make_persistent_key(zend_string *path);
zend_op_array *persistent_compile_file(zend_file_handle *file_handle, int type);

/* Epoch-based reclamation API (GH-8739) */
void accel_epoch_init(void);
void accel_epoch_enter(void);
void accel_epoch_leave(void);
bool accel_deferred_reset_pending(void);
void accel_try_complete_deferred_reset(void);

#define IS_ACCEL_INTERNED(str) \
((char*)(str) >= (char*)ZCSG(interned_strings).start && (char*)(str) < (char*)ZCSG(interned_strings).top)

Expand Down
Loading
Loading