fix: add rwlock for implicit opcache restarts (OOM/hash overflow)#2352
Open
superdav42 wants to merge 1 commit intophp:fix/opcache-safe-resetfrom
Open
fix: add rwlock for implicit opcache restarts (OOM/hash overflow)#2352superdav42 wants to merge 1 commit intophp:fix/opcache-safe-resetfrom
superdav42 wants to merge 1 commit intophp:fix/opcache-safe-resetfrom
Conversation
PR php#2073 drains all threads for explicit opcache_reset() calls, but WordPress triggers implicit restarts via opcache_invalidate() filling opcache memory → zend_accel_schedule_restart() → restart_pending → the actual reset runs during the next php_request_startup(). That path bypasses the opcache_reset() override and races on shared memory. Add a pthread_rwlock around the request lifecycle: - Normal requests: read lock (concurrent, single atomic CAS) - When zend_accel_schedule_restart_hook fires: set an atomic flag - Next php_request_startup(): sees flag → write lock (exclusive), blocks until all current requests complete, resets safely - After startup: unlock, all threads resume Combined with PR php#2073, this covers both explicit opcache_reset() (thread drain) and implicit OOM/hash-overflow restarts (rwlock). Testing: without this patch, crashes every ~10 min on a WordPress multisite. With PR php#2073 alone, same crash frequency. With the rwlock from our PR php#2349 alone, crashes reduced to every 6-12 hours (worker threads not covered). Combined fix expected to eliminate both paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
pthread_rwlockaround the request lifecycle to protect against implicit opcache restarts (OOM, hash overflow) that bypass theopcache_reset()override in #2073.Problem
PR #2073 intercepts explicit
opcache_reset()calls and drains all threads before the reset. However, WordPress (and other apps) trigger implicit opcache restarts through a different path:opcache_invalidate()fills opcache memory over timezend_accel_schedule_restart(ACCEL_RESTART_OOM)restart_pending = truephp_request_startup()→accel_activate()seesrestart_pending→accel_is_inactive()→ proceeds withaccel_interned_strings_restore_state()(memset on shared memory)This path never calls
opcache_reset(), so #2073's override doesn't fire.Production evidence
Fix
Uses PHP's
zend_accel_schedule_restart_hook(PHP 8.4+, declared inZend/zend.h) to detect when an implicit restart is scheduled, then serializes the nextphp_request_startup()via apthread_rwlock:php_request_startup(): sees flag → acquires write lock (exclusive), blocks until all current requests finishphp_request_shutdown()and release their read locksaccel_is_inactive()correctly sees no active threads, reset proceeds safelyWhy both fixes are needed
opcache_reset()calls — drains threads via Go state machine, works on all PHP versionsThe rwlock is unconditional (all PHP versions get the lock around requests) but the hook registration is
#if PHP_VERSION_ID >= 80400. On PHP 8.2/8.3, the rwlock adds negligible overhead with no functional benefit for implicit restarts.Changes
frankenphp.c: +50 linespthread_rwlock_t frankenphp_opcache_rwlock— read-write lockfrankenphp_opcache_restart_hook()— sets atomic flag (PHP 8.4+)php_thread()loop: read lock beforephp_request_startup(), unlock afterphp_request_shutdown(), write lock when restart flag is setfrankenphp_sapi_module.startup()