Skip to content

Fix GH-8739: OPcache restart corrupts SHM in threaded SAPIs - #24276

Open
pavimus wants to merge 2 commits into
php:PHP-8.4from
pavimus:fix-gh8739-zts
Open

Fix GH-8739: OPcache restart corrupts SHM in threaded SAPIs#24276
pavimus wants to merge 2 commits into
php:PHP-8.4from
pavimus:fix-gh8739-zts

Conversation

@pavimus

@pavimus pavimus commented Oct 12, 2026

Copy link
Copy Markdown

Prepared with an AI assistant (Claude), which also ran the reproduction and the tests on my machine; I reviewed the result. The text below is AI-generated (quoted, as asked in CONTRIBUTING.md).

We run PHP 8.5 ZTS under FrankenPHP and hit crashes on every OPcache restart under load. This PR fixes the restart race for threaded SAPIs, including multi-process ones (Apache event/worker), building on the approach discussed in #22281 by @nacholibre, who is credited as co-author of the main commit.

Problem

Readers of the OPcache SHM are tracked with an fcntl() read lock on byte 1 of the lock file,
and a scheduled restart (OOM, hash overflow, opcache_reset()) runs in RINIT only when
accel_is_inactive() finds no reader. POSIX record locks belong to the process, so under ZTS:

  1. F_GETLK ignores the locks of the calling process: threads of the restarting process are
    invisible and the restart wipes SHM while they execute code from it (String corruption on deployments with php-fpm 8.1.6 #8739 for threaded
    SAPIs; FrankenPHP: Crash with "zend_mm_heap corrupted" upon Symfony deploy and cache:clear (classic mode) frankenphp#2170, FrankenPHP crashes with zend_mm_heap corrupted while running some wordpress sites frankenphp#1737).
  2. Any thread finishing its request calls accel_unlock_all(), which unlocks the whole file and
    thereby the reader lock of the entire process; with several multi-threaded processes (Apache
    event/worker) another process then sees no reader. (It also drops the SHM write lock of other
    threads — fixed by the first commit, see below.)

Signatures: zend_mm_heap corrupted, SIGSEGV with opline->handler == NULL, garbage in
op_array/prev_execute_data.

Model

As suggested by @morrisonlevi in #22281: Windows — atomic model only (unchanged); POSIX NTS —
process model only (unchanged); POSIX ZTS — both. Threads of a process are counted in a
process-local atomic counter, and the process holds the fcntl() reader lock on byte 1 whenever
the counter is not zero, so other processes still see it and the kernel still releases it when a
process dies. This is the composition @nacholibre proposed in #22281 on 2026-06-18.

Changes (all under #if defined(ZTS) && !defined(ZEND_WIN32), ZendAccelerator.c only)

  • One acquisition point. A request becomes a reader only in RINIT, right after the restart
    block and before the last_restart_time check, and stays one until post_deactivate
    (ZCG(counted), as on Windows). Lazy acquisition (accel_activate_add()) fails under ZTS, so
    a request that was not admitted never starts to use SHM later (file cache, interned strings).
  • Counter. Enter: CAS "increment only if > 0"; otherwise, under a process-local mutex,
    F_SETLK F_RDLCK byte 1 and count 0 -> 1. Leave: CAS "decrement only if > 1"; otherwise, under
    the mutex, 1 -> 0 and F_UNLCK byte 1. fcntl() and the mutex are only used on 0 <-> 1.
  • Restart gate. The restarting thread (holding zend_shared_alloc_lock()) takes the mutex,
    requires a zero counter and a non-blocking F_SETLK F_WRLCK on byte 1, and holds both until
    the restart is done. No thread of the process can become a reader (the counter only grows from
    zero under the mutex), and the kernel neither grants the write lock while another process holds
    the reader lock nor the reader lock while the write lock is held. Check and acquisition are one
    kernel operation, so there is no window between "no readers" and "restart starts" (the stock
    F_GETLK probe, and Fix GH-8739: OPcache restart corrupts SHM in threaded SAPIs (ZTS) #22281, check first and lock later).
  • accel_unlock_all() releases only the thread's reference under ZTS, never the whole file.
  • force_restart_timeout. Other processes are still killed by kill_all_lockers(). Threads
    of the own process cannot be killed: a warning (PID, number of threads, delay) is logged at most
    once per timeout period.
  • Thread retirement. If a thread is destroyed while still counted (its post_deactivate was
    skipped), the globals destructor releases its reference (using only the passed globals:
    ts_free_id() runs destructors of all threads from one thread).
  • Builds without real atomics (HAVE_NO_ATOMICS) skip the lock-free paths and always take the
    mutex.

No ABI change: no change to zend_accel_globals/zend_accel_shared_globals, no new
ZEND_API symbol, no change to Zend/zend_atomic.h (the existing load/store/CAS are enough).
No lock on the read path: one uncontended CAS per request on enter and on leave.
NTS and Windows: no change in the compiled code (we compared ZendAccelerator.o of NTS debug
builds: only __LINE__ immediates differ).

Behaviour while a restart waits

ZCSG(accelerator_enabled) is false: new requests run uncached and are not counted, so the restart
runs as soon as the requests that were running when it was scheduled are done. A thread that wants
to become a reader while the restart itself runs waits on the mutex for its duration.

FrankenPHP worker mode: OPcache RINIT/post_deactivate run once per worker lifetime and a
worker keeps classes from SHM between HTTP requests, so a worker is a reader for its whole life
and a restart waits until workers are restarted (classic requests run uncached meanwhile). Not
counting workers would bring the crash back. The SAPI can restart its workers asynchronously from
zend_accel_schedule_restart_hook.

Known limitations

  • Tracing JIT: on AArch64 restarts with tracing JIT also crashed because of a separate JIT
    bug (stub veneers are not reset by zend_jit_restart()), fixed in Fix JIT stub veneers being reused after an OPcache restart on AArch64 #24273. With both fixes we
    still saw rare tracing-JIT-only issues under ZTS (3/40 runs where the restart did not happen
    within 40 s, 1/10 crash on Apache) that we have not diagnosed yet. Without JIT and with
    opcache.jit=function we saw none.
  • Preloading + JIT: requests that start while a restart is pending are not counted but still
    run preloaded functions, including JIT code that zend_jit_restart() rewinds (this also applies
    across NTS processes today). Out of scope here.
  • pcntl_fork() from a request with active readers is not supported (the child copies a positive
    counter but not the lock).

Testing

Reproduction harness (Docker; FrankenPHP 1.13.0 classic/worker built from source against this
branch, Apache 2.4 event MPM 3-4 children x 32 threads with mod_php ZTS; a generated app of 2520
class files with inheritance, 50-500 ms requests; opcache.memory_consumption=32, SHM filled under
load, then files touched every 5 s to force OOM restarts, or opcache_reset()). AArch64 Linux.

Scenario stock PHP-8.4 #22281 this PR
FrankenPHP 32 threads, OOM restarts 5/5 crash 0/5 0/10 (40 restarts)
same, opcache.jit=function — — 0/10 (40 restarts)
FrankenPHP, hash overflow restarts 5/5 crash 0/5 0/10
FrankenPHP worker mode + classic requests 5/5 crash no restart no restart (by design, see above)
FrankenPHP 1 thread (control) 0/5 0/5 0/5
Apache event, OOM restarts 5/5 crash 5/5 crash 0/10 (40 restarts)
Apache event, opcache_reset() every 5 s 5/5 crash 5/5 crash 0/10 (70 restarts)
Apache event, a child killed with SIGKILL before every reset — — 0/3, 21/21 restarts done
45 min each: FrankenPHP 8..64 threads with load bursts (69 threads started, 13 retired) / Apache event, file edits every 8 s — — no crash, 146 / 105 restarts

make test for ext/opcache and Zend/tests (5881 tests): identical results to stock on ZTS and
NTS debug builds (0 failures). Builds with --enable-werror, also with --disable-opcache-jit.
Merges into PHP-8.5 cleanly, into PHP-8.6 with a trivial conflict; master needs C11 atomics instead
of zend_atomic (branch available).

Performance (FrankenPHP classic, 32 threads, wrk, builds interleaved): hello world at 64
connections 81.3k (stock) vs 81.8k RPS (5 rounds, medians); at 1 connection — every request takes
the 0 <-> 1 path with the mutex and two fcntl() — 16.1k vs 16.2k RPS. Under contention stock pays
two fcntl() per request, this PR none on the fast path.

Relation to other work

🤖 Generated with Claude Code

pavimus and others added 2 commits October 12, 2026 13:28
@pavimus
Fix OPcache releasing other threads' SHM locks under ZTS
02fdf5a 
accel_unlock_all() runs at the end of every request and releases the
whole lock file with F_UNLCK (l_start = 0, l_len = 0). fcntl() record
locks are owned by the process, not by the thread, so under ZTS a
thread finishing its request also releases the SHM write lock (byte 0)
held by another thread of the same process inside
zend_shared_alloc_lock(), and the restart lock (byte 2).

With several multi-threaded processes sharing the cache (Apache
mod_php with the event or worker MPM) another process can then take
the write lock while the first one is still writing, and two processes
modify SHM at the same time. Children crash during warm-up even
without any OPcache restart (SIGSEGV in accel_find_interned_string()
and zend_hash_find_bucket()).

Bytes 0 and 2 are always released by the thread that took them
(zend_shared_alloc_unlock(), zend_shared_alloc_safe_unlock(),
accel_restart_leave()), so under ZTS only release the reader lock.
NTS is unchanged.
@pavimus @nacholibre
Fix phpGH-8739: OPcache restart corrupts SHM in threaded SAPIs
5492ce3 
Readers of the shared memory are tracked with an fcntl() read lock on
byte 1 of the lock file, and a restart only runs when no process holds
it. Record locks belong to the process, so under ZTS this does not see
threads: F_GETLK ignores the locks of the calling process, and the
restart (OOM, hash overflow, opcache_reset()) wipes SHM while other
threads of the same process still execute code from it. With several
multi-threaded processes (Apache event/worker MPM) any thread ending
its request also released the reader lock of the whole process.
Result: crashes ("zend_mm_heap corrupted", SIGSEGV) under FrankenPHP
and Apache mod_php ZTS.

Under ZTS on POSIX (NTS and Windows are unchanged):

- A thread becomes a reader only in RINIT, before the restart
  generation is checked, and stays one until post_deactivate. The
  threads of a process are counted in a process-local atomic counter;
  the process holds the reader lock on byte 1 whenever the counter is
  not zero. Only the 0 <-> 1 transitions take a mutex and call fcntl(),
  other requests do a single compare-and-swap on enter and on leave.
- Lazy reader locks (accel_activate_add()) fail under ZTS, so a request
  that was not admitted in RINIT never starts to use SHM later (e.g.
  through the file cache or interned strings).
- A restart takes the mutex, requires a zero counter and a write lock
  on byte 1, and holds both until it is finished. Threads of the
  process cannot become readers meanwhile, and the kernel neither
  grants the write lock while another process holds the reader lock
  nor the reader lock while the write lock is held. This replaces the
  F_GETLK probe, so there is no window between checking for readers
  and starting the restart.
- Threads cannot be killed after opcache.force_restart_timeout: a
  warning is logged once per timeout period instead. Other processes
  are still killed by kill_all_lockers().
- A thread retired without its request cleanup releases its reference
  in the globals destructor.
- Without real atomics (HAVE_NO_ATOMICS) every enter and leave takes
  the mutex.

FrankenPHP worker threads stay readers for their whole life, so a
restart waits until the workers are restarted. Not covered: requests
that start while a restart is pending still run preloaded functions,
including their JIT code.

Co-authored-by: Yoan Arnaudov <1738437+nacholibre@users.noreply.github.com>
@pavimus
pavimus requested a review from dstogov as a code owner October 12, 2026 14:00
This was referenced Oct 12, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dstogov dstogov Awaiting requested review from dstogov dstogov is a code owner

Assignees

No one assigned

Labels

Extension: opcache

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@pavimus