feat(async): event-loop async support — fd primitive, adapters, native Swoole, shared reactor - #138
Merged
Merged
Conversation
…e Swoole, shared reactor Implements #24: let the driver participate in non-blocking PHP event loops instead of only blocking on Future::get(). Core primitive (C, src/FutureNotifier.{c,h}): - Every Future gains getResource(): resource and isReady(): bool. getResource() returns a php_stream that becomes readable once, when the driver resolves the future on its IO thread (cass_future_set_callback + self-pipe). Any loop (stream_select / ReactPHP / AMPHP / Swoole) can await without blocking; get() stays blocking, so sync users pay nothing. - Refcounted, persistent-malloc notifier. Linux eventfd fast path (1 syscall, 1 fd, no SIGPIPE) with a POSIX pipe fallback on macOS; SIGPIPE-safe dup for the stream; register-then-recheck closes lost-wakeup races. Framework adapters (lib/Async, optional peer deps — require-dev + suggest): - Revolt (fiber await, default), Amp (toFuture), ReactPhp (toPromise), Swoole (coroutine waitEvent). Native Swoole/OpenSwoole (opt-in build flag): - -DPHP_SCYLLADB_ENABLE_SWOOLE / _OPENSWOOLE compiles one C++ shim (src/Async/SwooleBridge.cc) so Future::get() suspends the current coroutine instead of blocking. Default build stays pure C; swoole symbols resolve lazily. Shared reactor (opt-in, O(1) fds for high fan-out): - Cassandra\Async\Reactor + src/Async/Reactor.{c,h}: one eventfd + mutex-guarded MPSC completion queue shared across many futures, so the loop watches ONE fd regardless of concurrency. fd+mutex are module/thread-lifetime; registration state resets each request. lib/Async/ReactorRevolt.php adapter. Tests + benchmarks: - tests/Feature/Async/*: primitive, Revolt/Amp/React/Swoole (guarded), reactor (io_threads=4 multi-producer, error propagation, model-mixing guards). - benchmarks/Live/{AsyncBench,ReactorBench}.php + committed baselines under benchmarks/baselines/ (JSON + Markdown) via composer bench:baseline:export. - docs/async.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…guard - FutureNotifier.c: reorder notifier_ensure so the register-then-recheck poke happens BEFORE dropping the callback ref, not after. Functionally identical (refcount is 2 across the poke, never freed), but removes the path clang-tidy's analyzer flagged as use-after-free (clang-analyzer-unix.Malloc). - SwooleBridge.cc: wrap the whole TU in #ifdef HAVE_SWOOLE_COROUTINE so tooling that analyzes the default build (clang-tidy) doesn't choke on the absent <swoole.h> — the file is only compiled when the swoole build flag is set. - ReactorTest: "surfaces a failed query" now loops until its future is dispatched instead of asserting a single select+poll. The reactor fd is level-triggered and can report a spurious wake, so the single-shot form was flaky across tests (leaked a pending future into the next test on CI). - test.yml (pre-existing ASan failure, unrelated to async): disable opcache in the ASan job — PHP dlopen()s opcache with RTLD_DEEPBIND, which the sanitizer runtime rejects and aborts on before any test runs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…DEEPBIND exts under ASan - FutureNotifier.c: on the set_callback-failed path drop the callback ref with a direct atomic decrement instead of unref(). Refcount is provably 2→1 there (creation ref always survives), so it can never free; the inline decrement stops clang-analyzer-unix.Malloc from modelling an impossible free of the pointer returned in *notifier_slot. - Reactor.c: reactor_consume now drops the reg's two refs (ready-list + registered) in a single atomic sub-2 via reg_release() instead of two reg_unref() calls, so the analyzer no longer sees a deref-after-free between the two decrements. Verified clean with clang-analyzer-unix.Malloc locally. - test.yml ASan job: also disable mysqli/pdo_mysql/mysqlnd (pre-existing; the 8.4 runner dlopen()s mysqlnd with RTLD_DEEPBIND, same abort as opcache). The 8.5 ASan job already runs the full suite (incl. reactor) clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reverts the opcache/mysql extension-disabling in the ASan job. On PHP 8.4 the runner dlopen()s opcache/mysqlnd with RTLD_DEEPBIND (incompatible with the sanitizer runtime → abort before tests) — a pre-existing failure that also fails on trunk and is unrelated to this PR. Disabling those extensions to work around it broke the 8.4/8.5 mysqli↔mysqlnd dependency at startup, so it's not a net improvement. Left as-is for a dedicated CI-infra change; the 8.5 ASan job already exercises the full suite (incl. the reactor) memory-safety clean. The C-side clang-tidy fixes (FutureNotifier direct decrement, Reactor reg_release) are retained. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
PHP 8.4's runner dlopen()s opcache/mysqlnd with RTLD_DEEPBIND, which the sanitizer runtime rejects (aborts before any test runs) — a pre-existing failure (also red on trunk). Disable those extensions for 8.4 only via a matrix- conditional so the 8.4 ASan run can start; 8.5 is left untouched (it doesn't hit the abort, and disabling mysqlnd there breaks the mysqli↔mysqlnd startup dep). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…fra issue Disabling extensions one-by-one on the 8.4 ASan job is whack-a-mole: the runner dlopen()s opcache, mysqlnd, pdo, … with RTLD_DEEPBIND, which the sanitizer runtime rejects. This is a fundamental 8.4-runner/ASan incompatibility that also fails on trunk and is unrelated to this PR; it needs a dedicated CI change (e.g. drop 8.4 from the ASan matrix, or build PHP without RTLD_DEEPBIND). Leaving test.yml untouched. The 8.5 ASan job already validates this PR's memory safety. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Add a `swoole` CI job (matrix: swoole@8.3/8.4, openswoole@8.3) that builds the extension with -DPHP_SCYLLADB_ENABLE_SWOOLE/_OPENSWOOLE against the runtime's own source headers (checked out at the installed version for ABI match), loads the coroutine runtime, and runs the swoole test group against live ScyllaDB. - lib/Async/Swoole.php: make the adapter flavor-aware — detect Swoole\ vs OpenSwoole\ Coroutine/System at runtime and use the runtime's EVENT_READ constant instead of a hardcoded value. - SwooleTest: flavor-aware coroutine runner; cover native coroutine-aware get(), the adapter, and many in-flight futures. Runs when swoole/openswoole is loaded. - test.yml: add lib/** to the path filter so async userland changes trigger CI. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The CI swoole jobs failed with `undefined symbol: php_scylladb_swoole_current_cid`. Two causes: - The #ifdef HAVE_SWOOLE_COROUTINE guard on SwooleBridge.cc compiled it to an empty object under GCC (the target define wasn't reaching the .cc), so the shim symbols were absent. The guard was only for clang-tidy; drop it and instead exclude the file from the clang-tidy file list (it needs the swoole source headers to parse). - OpenSwoole renamed its headers (openswoole_*.h), C++ namespace (`openswoole`), and event flag (OSW_EVENT_READ). Select the runtime with __has_include(<openswoole.h>) (keyed on the include path, not a define) + a namespace alias, so one shim builds against either tree. Also: don't re-load the coroutine ext via -d in the test run (setup-php already loads it via ini — avoids a "module already loaded" warning). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… fresh configure The project declares LANGUAGES C. CXX is otherwise enabled late by the legacy .cpp modules added after src/. This subdir is configured before them, so a fresh configure silently drops SwooleBridge.cc from the src target and the native shim symbols (php_scylladb_swoole_current_cid) are left undefined. Call enable_language(CXX) in the swoole block.
CodeLieutenant
force-pushed
the
claude/scylladb-php-async-support-904e8f
branch
from
August 5, 2026 07:16
6116998 to
51deb3a
Compare
CodeLieutenant
marked this pull request as ready for review
August 8, 2026 10:28
|
Tick the box to add this pull request to the merge queue (same as
|
There was a problem hiding this comment.
Pull request overview
Adds non-blocking async/event-loop support to the ScyllaDB PHP driver by exposing a “readable-on-completion” stream for futures, plus optional framework adapters and an opt-in shared reactor to reduce FD usage under high concurrency. This enables awaiting query completion inside ReactPHP/Amp/Revolt/Swoole loops without blocking, while keeping Future::get() blocking behavior unchanged for sync users (except the opt-in native Swoole build path).
Changes:
- Extend
Cassandra\Future(and concrete Future types) withgetResource()andisReady()backed by a new C notifier (eventfdon Linux, pipe fallback elsewhere). - Add
Cassandra\Async\Reactor(shared eventfd + MPSC completion queue) and Revolt adapter for O(1) watcher scaling. - Add optional userland adapters/tests/benchmarks/docs plus CI coverage for the opt-in native (Open)Swoole coroutine build.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Feature/Async/SwooleTest.php | Adds feature tests for Swoole/OpenSwoole coroutine awaiting and native build behavior. |
| tests/Feature/Async/RevoltAwaitTest.php | Adds Revolt fiber-await feature tests (single + concurrent + error propagation). |
| tests/Feature/Async/ResourcePrimitiveTest.php | Adds framework-agnostic tests for Future::getResource() / isReady() semantics. |
| tests/Feature/Async/ReactPromiseTest.php | Adds ReactPHP promise adapter feature tests. |
| tests/Feature/Async/ReactorTest.php | Adds shared reactor feature tests (fan-out, MPSC IO threads, mixing guards). |
| tests/Feature/Async/AmpFutureTest.php | Adds Amp adapter feature tests. |
| src/php_scylladb.c | Wires reactor lifecycle into module globals (GINIT/GSHUTDOWN/RSHUTDOWN). |
| src/FutureValue.stub.php | Extends FutureValue API surface with getResource() / isReady(). |
| src/FutureValue.c | Implements FutureValue getResource()/isReady() and notifier lifetime management. |
| src/FutureSession.stub.php | Extends FutureSession API surface with getResource() / isReady(). |
| src/FutureSession.c | Makes get() coroutine-aware via notifier wait; adds getResource()/isReady(). |
| src/FutureRows.stub.php | Extends FutureRows API surface with getResource() / isReady(). |
| src/FutureRows.c | Adds resource primitive + reactor mixing guard + notifier cleanup for FutureRows. |
| src/FuturePreparedStatement.stub.php | Extends FuturePreparedStatement API surface with getResource() / isReady(). |
| src/FuturePreparedStatement.c | Makes get() coroutine-aware; adds getResource()/isReady() + notifier cleanup. |
| src/FutureNotifier.h | Introduces cross-thread completion notifier API (callback-safe, persistent allocation). |
| src/FutureNotifier.c | Implements notifier backend (eventfd/pipe), stream publication, and coroutine-aware waiting. |
| src/FutureClose.stub.php | Extends FutureClose API surface with getResource() / isReady(). |
| src/FutureClose.c | Makes get() coroutine-aware; adds getResource()/isReady() + notifier cleanup. |
| src/Future.stub.php | Extends Cassandra\Future interface with getResource() and isReady() docs/contract. |
| src/CMakeLists.txt | Adds reactor/notifier sources and opt-in native Swoole C++ bridge build logic. |
| src/Async/SwooleBridge.h | Declares C-callable wrappers for Swoole coroutine wait APIs (guarded by build flag). |
| src/Async/SwooleBridge.cc | Implements native coroutine waiting shim for (Open)Swoole (only C++ TU). |
| src/Async/Reactor.stub.php | Adds PHP stub for Cassandra\Async\Reactor API. |
| src/Async/Reactor.h | Declares reactor lifecycle functions for module init/shutdown integration. |
| src/Async/Reactor.c | Implements shared reactor (single fd + ready queue) and PHP-facing static methods. |
| lib/Async/Swoole.php | Adds userland Swoole/OpenSwoole await adapter using waitEvent + getResource(). |
| lib/Async/Revolt.php | Adds Revolt-based fiber await adapter (await/awaitAll) built on onReadable watcher. |
| lib/Async/ReactPhp.php | Adds ReactPHP adapter converting a Future to a Promise settled on fd readability. |
| lib/Async/ReactorRevolt.php | Adds Revolt adapter for the shared reactor (single watcher dispatching many futures). |
| lib/Async/Amp.php | Adds Amp v3 adapter converting a Future to an Amp\Future via Revolt watcher. |
| include/php_scylladb_types.h | Extends Future structs with notifier/stream fields and reactor registration pointer. |
| include/php_scylladb_globals.h | Adds reactor pointer to module globals (forward-declared). |
| docs/async.md | Documents the async primitive, adapters, native Swoole build, and shared reactor usage/limits. |
| composer.lock | Adds dev dependencies for Revolt/Amp/React used by adapters and tests. |
| composer.json | Adds adapter autoload mapping, suggests optional deps, and introduces benchmark export script. |
| CMakeLists.txt | Adds top-level CMake options for native Swoole/OpenSwoole build configuration. |
| benchmarks/Live/ReactorBench.php | Adds live benchmark comparing per-future fds vs shared reactor at varying fan-out. |
| benchmarks/Live/AsyncBench.php | Adds an event-loop-style benchmark using getResource() + stream_select. |
| benchmarks/baselines/README.md | Adds committed baseline documentation for PhpBench regression tracking. |
| benchmarks/baselines/php85-offline.json | Adds committed offline benchmark baseline data. |
| benchmarks/baselines/php85-live.json | Adds committed live benchmark baseline data. |
| benchmarks/baseline.sh | Adds helper script to export baselines (best-effort live group, always offline). |
| benchmarks/baseline.php | Adds parser to convert PhpBench XML dumps into portable JSON + Markdown summaries. |
| .github/workflows/test.yml | Expands CI triggers to lib/** and adds a dedicated native Swoole/OpenSwoole job. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…async-support-904e8f
Adds Cassandra\Async\PollHandle, a driver notification descriptor as a native Io\Poll\Handle, and Cassandra\Async\Poll, a loop over Io\Poll\Context that watches futures and drops each watcher as its future resolves. A driver descriptor stays readable after it fires, so a watcher left in place makes the loop spin. Io\Poll\Context::add() takes an Io\Poll\Handle, and 8.6 ships no handle class for a plain PHP stream, so a driver descriptor cannot reach the polling API without one. The handle also skips a layer: it registers the driver's own descriptor, with no dup() and no stream resource per watched future. Off by default. PHP 8.6 is unreleased and Io\Poll\Context::wait() changed shape after 8.6.0alpha3, so a default-on build would track a moving target. Build with -DPHP_SCYLLADB_ENABLE_POLL_API=ON, or --enable-poll-api through pie or pecl. The class registry now resolves an external dependency in CG(class_table) instead of zend_lookup_class(). The latter reads EG(class_table), which init_executor() only fills at request start, so at MINIT it dereferences a null table. Io\Poll\Handle comes from ext/standard, whose MINIT runs first.
The Revolt, Amp, ReactPHP and Swoole adapters are userland PHP with framework dependencies the extension itself does not need. They move from lib/Async to packages/async-adapters, published as codelieutenant/scylla-driver-async-adapters and pulled in here through a path repository. Installing the driver no longer implies installing a framework, and a user who wants one adapter no longer carries all four.
FutureRows::get() caches the decoded row array and hands every Rows object a refcounted copy of it, so the array's own internal pointer was shared between the future and every Rows built from it. Iterating one moved the other. A debug PHP aborts on the ZEND_ASSERT in zend_hash_internal_pointer_reset_ex, and a release build silently shares the cursor. The async work makes this the normal path: a loop holds futures alive and calls get() on each as it resolves, which is exactly the case that shares the array. Rows now carries its own HashPosition and uses the _ex iteration API. Rows::first() is fixed by the same change. It reset a local position and then read through the internal pointer, so after any iteration it returned the current row instead of the first one.
Reactor::add() took Cassandra\Future but threw for anything except FutureRows. It now accepts every concrete future, so a boot sequence can fan out prepareAsync() the same way a request fans out executeAsync(). A FutureValue resolves on construction and goes straight to the ready list. Each future keeps its notifier and its reactor registration at its own struct offset. One resolver in FutureNotifier.c now knows where, and the reactor, the Io\Poll handle and the poll loop share it instead of each carrying a copy of the class ladder. A registration records the resolved CassFuture and a pointer to the future's own slot, so dispatch and drain no longer re-derive either from the class. The "one async model per future" guard is shared too, and now covers all five types in both directions. Two contract fixes reported by review on #138: - cass_future_set_callback() failing on a still-pending future left the descriptor readable with nothing to read. poll() would hand back a future whose get() blocks. Both the notifier and the reactor now roll the registration back and throw, and only self-signal a future that has already resolved. - A callback that throws makes poll() return nothing, which dropped whatever that call had already collected. Those futures were unregistered but never handed to userland, so they are registered again at the front of the queue and the next poll() returns them first. Under a native Swoole build, get() on a reactor-registered future tried to install a second driver callback. wait_coro() now takes the registration and falls back to the plain timed wait, which does not block: poll() only returns futures that have already resolved.
Nothing in the matrix built against PHP 8.6, so Cassandra\Async\Poll and PollHandle had no coverage at all. Adds a job that does, and a build-extension input to drive the flag. The job builds with PHP_SCYLLADB_ENABLE_POLL_API=ON rather than AUTO on purpose: AUTO against a PHP without the header produces an extension with no Poll classes and a test file that skips itself green. ON fails the build instead. The job asserts the classes exist before it runs the suite. It reports without gating (continue-on-error), because 8.6 is unreleased and Io\Poll\Context::wait() is still changing shape. The push and pull_request path filters watched lib/**, which no longer exists after the adapters moved to packages/.
Reported by review on #138: ReactorBench declared ParamProviders at class level and again on benchSharedReactorHighFanout. PhpBench combines providers, and both used the 'requests' key, so the high-fanout subject produced duplicate variants and duplicate baseline rows. Each subject now names the provider it wants, which gives 64/256 twice and 1024/4096 once. docs/async.md said a throwing callback loses nothing only for the queued remainder, which was not true for the completions poll() had already collected, and it offered Io\Poll\StreamPollHandle as an alternative to PollHandle. No such class exists: Io\Poll\Context::add() takes an Io\Poll\Handle, and 8.6 ships no handle for a plain PHP stream. Both are corrected, along with the note that the reactor now takes every future type.
The guide and the reference both stated that the driver has no callback and no event loop integration. That has not been true since the completion descriptor landed, and a reader following either page would conclude the feature does not exist. Adds guide/event-loops.md, which covers the getResource()/isReady() primitive and its contract, stream_select() without a framework, the adapter package, the shared reactor and its batching, native Io\Poll on 8.6, and coroutine-aware get() under a native Swoole build. Adds reference/async.md for Reactor, Poll and PollHandle. Corrects the two stale sections and wires both pages into the sidebar.
Replace the integral macros in InetUtil.c with an enum, and read the address fields with strtoul() and strtol() instead of sscanf() and atoi(). The tokenizer only emits validated digits, so the values are unchanged. Remove the redundant casts around export_twos_complement() and around the tuple index lookup. Drop the gnu::nonnull attribute from the FutureUtil declarations. The driver returns a null future on an allocation failure, and both functions report that as a PHP exception, so the attribute let the optimizer delete the guard. Zero-initialize the zvals in TypeFactory.c. ZVAL_UNDEF only writes the type tag, so the value union stayed indeterminate in zvals that the code copies and returns by value.
php_scylladb_format_decimal() asserted that the scale is not negative.
A positive exponent makes the scale negative, so new Decimal('1e3')
aborted a debug build and produced a wrong string in a release build.
Print a negative scale in scientific notation, which is the rule that
BigDecimal.toString() uses. ScyllaDB stores the CQL literal 1E+3 as the
same pair, an unscaled 1 with a scale of -3, and the round trip through
the wire keeps both fields.
Size the output buffer after the digit count is known. The old
allocation added the scale to the length, so a negative scale made the
buffer too small for the digits.
Copy only the fractional digits when an exponent follows them. The old
length also copied the 'e', so '1.5e2' failed to parse.
Parse the exponent with strtol() and check the range. sscanf() with %d
reports a read failure as EOF, which the old test accepted, and it
overflows without a report. This also clears a clang-tidy finding.
strtol() stops at the first character it cannot use and skips leading whitespace, so "1e2foo" and "1e 2" parsed as 1e2. Require the exponent token to start with a digit or a minus sign, and to end at the end of the string. Also stop the clang-tidy job from linting Poll.c and PollHandle.c: they need main/php_poll.h, which only PHP 8.6 ships, and the job builds against PHP 8.5. Run the PHP 8.6 Io\\Poll job on demand only until 8.6 is stable.
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.
Implements #24 — makes the driver usable inside non-blocking PHP event loops instead of only blocking on
Future::get().The C driver resolves futures on its own IO thread(s); this exposes that completion to userland so any loop can await a query without blocking.
get()stays blocking, so existing sync code is unaffected and pays nothing.What's in it
1. Core primitive (C) — works with any loop, zero deps
Every
Futuregains:getResource()returns aphp_streamthat becomes readable exactly once when the driver resolves the future (viacass_future_set_callback+ a self-pipe). Watch it withstream_select/ReactPHP/AMPHP/Swoole; on readable,get()returns without blocking. Linux useseventfd(1 syscall, 1 fd, no SIGPIPE); macOS falls back to a pipe.src/FutureNotifier.{c,h}.2. Framework adapters (
lib/Async/, optional peer deps —require-dev+suggest)Revolt(fiber await, the default),Amp(toFuture),ReactPhp(toPromise),Swoole(coroutinewaitEvent).3. Native Swoole/OpenSwoole — opt-in build flag
-DPHP_SCYLLADB_ENABLE_SWOOLE/_OPENSWOOLE(+-DPHP_SCYLLADB_SWOOLE_SRC=…) compiles one small C++ shim (src/Async/SwooleBridge.cc) soFuture::get()suspends the current coroutine instead of blocking. Default build stays pure C; swoole symbols resolve lazily at runtime.4. Shared reactor — opt-in, O(1) fds for high fan-out
Cassandra\Async\Reactor(::add/resource/poll/pending): one eventfd + a mutex-guarded MPSC completion queue shared across many futures, so the loop watches one fd regardless of concurrency. Per-future fds cap out atFD_SETSIZE(~512 on macOS); the reactor scales to thousands and is even a touch faster at matched fan-out.lib/Async/ReactorRevolt.phpadapter. Seedocs/async.md.Testing
tests/Feature/Async/*against live ScyllaDB: primitive (stream_selectmultiplexing, error propagation, early-free SIGPIPE regression), Revolt/Amp/React (concurrency), Swoole (skip-guarded), reactor (io_threads=4multi-producer, errors, model-mixing guards, Revolt adapter). All green; full suite 880 passed.benchmarks/Live/{AsyncBench,ReactorBench}.php+ committed baselines underbenchmarks/baselines/(JSON + Markdown, viacomposer bench:baseline:export).Notes for reviewers
io_threads=4stress + leak checks).*_arginfo.h/*_descriptor.care build-time artifacts (git-ignored), regenerated from the committed stubs.UuidTest › unique UUIDs across separate processes) is environmental — it spawnsphp -d extension=cassandrachildren by bare name and passes in CI where the extension is installed.🤖 Generated with Claude Code