Skip to content

fix(reactor): no PHP object after the object store is gone (#264) - #265

Merged
EdmondDantes merged 2 commits into
mainfrom
264-exception-after-object-store
Aug 23, 2026
Merged

fix(reactor): no PHP object after the object store is gone (#264)#265
EdmondDantes merged 2 commits into
mainfrom
264-exception-after-object-store

Conversation

@EdmondDantes

@EdmondDantesEdmondDantes commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Closes#264.

async_new_exception returns NULL while EG(active) is 0, and the four call
sites in libuv_reactor.c that released its result unconditionally take that
NULL: :2204, :2870, :3126, :6286. The other 23 in that file already
carry NULL onward — assigned to req->base.exception and passed to NOTIFY, or
released under a check — and async_rethrow_exception(NULL) is a no-op on both
of its branches.

The window this guards

zend_deactivate runs shutdown_executor() and only then
ZEND_ASYNC_ENGINE_SHUTDOWN(). By that point zend_shutdown_executor_values
has cleared EG(active) ("No PHP callback functions should be called after this
point") and zend_objects_store_destroy has freed the bucket table.
libuv_reactor_shutdown still turns the loop, so a write libuv cancels there
reaches io_pipe_writev_cb with status != 0 and the old code built an
exception object with no store to put it in.

Nothing else runs in that window: every uv callback in this extension is defined
in libuv_reactor.c (scheduler.c mentions libuv only in comments), and the
listeners a NOTIFY reaches consume an exception rather than build one. Callers
outside that file run from PHP-visible code, where EG(active) is 1.

The test

tests/cleanup/005-write_cancelled_at_reactor_shutdown.phpt needs the shape
built on purpose: a queued write on a stream handle comes only from
ZEND_ASYNC_IO_WRITEV_EX, whose one consumer is true-async/server, and
php-src's single caller of the write API — main/streams/plain_wrapper.c:553
writes files, whose queued work the drain waits out instead of cancelling. So a
build configured with --enable-async-fuzz queues one such write at reactor
shutdown when ASYNC_FUZZ_CANCELLED_WRITE=1 is set, and the test skips on every
other build. run-tests hands a test's ENV section to its SKIPIF process as
well, so the hook declines to fire when TEST_PHP_EVALUATING_SKIPIF is set;
a putenv() in the SKIPIF cannot do it, because PHP restores the original
environment at request shutdown, before the reactor stops.

Without the guard the test prints its expected output and then segfaults, which
run-tests reports as Termsig=0; with it, 5 of 5 runs pass. The full suite on
this build: 1293 tests, 1192 passed, 98 skipped, 3 failed. All three failures
are include __DIR__ . '/../../../../ext/...' resolving outside php-src,
because this checkout is reached through a symlink; nothing in this change can
reach them.

Debug builds crash. Release builds take the fast-shutdown path
(shutdown_executor, #if ZEND_DEBUGfast_shutdown = 0), where the bucket
table outlives the teardown and the exception was built into a store nothing
reads again.

Where the symptom came from

true-async/server's h2/060-h2-write-deadline-reclaims failed 4 of 30
standalone runs and 2 of 17 full -j4 sweeps, its own assertions passing every
time. Against a PHP carrying this change: 0 of 30, and the server's phpt suite
is 349 passed, 20 skipped, 0 failed.

A second commit, test-only

pdo_mysql/011 failed on this branch's first CI run, and it is not this change:
four MySQL tests took the default fixture name async_test in one database, so
under parallel workers the DROP TABLE IF EXISTS of whichever started second
removed the table the first had just created. It reproduces on the PHP built on
14 August — 10 failing runs out of 10 over those four tests at -j4 — and 0 out
of 10 once the default becomes async_test_<pid>. It rides here because CI
cannot go green without it; the suite reaches MySQL only intermittently, which
is why the collision surfaced on this branch rather than earlier.

@EdmondDantes
EdmondDantesforce-pushed the 264-exception-after-object-store branch from 3edef8b to 8f2fb85CompareAugust 23, 2026 11:20
@codecov

codecovBot commented Aug 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

zend_deactivate runs shutdown_executor() before ZEND_ASYNC_ENGINE_SHUTDOWN(),
so the object store is freed and EG(active) is 0 by the time
libuv_reactor_shutdown turns the loop to finish cancelling work. A write libuv
cancels there reaches io_pipe_writev_cb, which branched on status == 0 alone
and sent every other status to async_new_exception, where object_init_ex wrote
into a NULL bucket table.
async_new_exception answers NULL while the executor is inactive, and the four
call sites that released its result unconditionally take that NULL.
The shape has no caller inside php-src, so a build configured with
--enable-async-fuzz queues one such write at reactor shutdown when
ASYNC_FUZZ_CANCELLED_WRITE=1 is set. Without the guard the new test prints its
expected output and then segfaults, which run-tests reports as Termsig=0; with
it, 5 of 5 runs pass. true-async/server's h2/060-h2-write-deadline-reclaims,
the intermittent symptom this started from, failed 4 of 30 standalone runs
before and 0 of 30 after.
@EdmondDantes
EdmondDantesforce-pushed the 264-exception-after-object-store branch from 8f2fb85 to c9669d3CompareAugust 23, 2026 11:56
… tests
createTestTable, cleanupTestTable and runAsyncTest defaulted to `async_test`
in both MySQL helpers, and every caller took the default. Under parallel
workers the DROP TABLE IF EXISTS of whichever test started second removed the
table the first had just created, and the first reported
`Table 'test.async_test' doesn't exist`.
The default is now `async_test_<pid>`: run-tests gives each test a process of
its own. Over 002, 004, 011 and 012 at -j4 — 10 failing runs out of 10 before,
0 out of 10 after; both MySQL groups, 39 tests, green.
The collision predates this branch: it reproduces the same way on the PHP built
on 14 August. It surfaced now because CI reaches MySQL only intermittently, and
a run that cannot connect skips these tests instead of racing them.
@EdmondDantes
EdmondDantes merged commit 12984eb into mainAug 23, 2026
2 of 8 checks passed
@EdmondDantes
EdmondDantes deleted the 264-exception-after-object-store branch August 23, 2026 12:28
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A cancelled write builds a PHP exception after the object store is gone

1 participant

@EdmondDantes