Skip to content

Fix OPcache releasing other threads' SHM locks under ZTS - #24272

Open
pavimus wants to merge 1 commit into
php:PHP-8.4from
pavimus:fix-zts-unlock-all
Open

pavimus wants to merge 1 commit into
php:PHP-8.4from
pavimus:fix-zts-unlock-all

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).

While working on GH-8739 for threaded SAPIs we found that Apache with a threaded MPM and mod_php ZTS crashes during warm-up even without any OPcache restart. This is a small, separate fix.

accel_unlock_all() runs at the end of every request (and in RINIT for a "stuck count") and
unlocks the whole lock file (l_start = 0, l_len = 0). fcntl() record locks belong to the
process, so under ZTS a thread ending its request also releases

  • byte 0, the SHM write lock that another thread of the same process holds inside
    zend_shared_alloc_lock() (that thread still holds zts_lock and keeps writing), and
  • byte 2, the restart lock.

With several multi-threaded processes sharing the cache (Apache event/worker MPM + mod_php ZTS)
another process can then take the write lock, and two processes modify SHM at the same time.

Evidence (AArch64 Linux, Apache 2.4 event, 3 children x 32 threads, 128M SHM, no restart):

  • A debug counter of threads inside zend_shared_alloc_lock(), kept in shared memory, saw two
    processes writing concurrently in 4 of 5 cold starts; with this change 0 of 5.
  • Children crashed before the first restart in 6 of 13 stock runs (SIGSEGV in
    accel_find_interned_string() on a garbage string table position, and in
    zend_hash_find_bucket() on a class method table); 0 of 13 with this change.

Bytes 0 and 2 are always released by the thread that took them (zend_shared_alloc_unlock(),
zend_shared_alloc_safe_unlock() in post_deactivate, accel_restart_leave()), so under ZTS only
the reader lock (byte 1) is released. NTS is unchanged.

This does not fix the restart race of GH-8739 (threads of the restarting process are still
invisible to the F_GETLK probe); that is a separate PR for GH-8739, which builds on this commit.

🤖 Generated with Claude Code

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant