From 3bd331d821c71a1e4c342fea9ebc761c8289beff Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:37:00 +0000 Subject: [PATCH 1/4] docs: record that breaking changes are allowed on the dev lines Every follow-up issue filed against this package hedged about backwards compatibility - proposing adapters, interim detection steps and deprecation paths to avoid touching published shapes. None of that is warranted: this package and the shared-data extension ride development lines with no external consumers, and LAYOUT_VERSION already hard-fails a mismatched reader, which is the only compatibility mechanism the family needs while it is being built. Stating it here because the hedging was the reasonable default in the absence of a written policy, and it will recur otherwise. The correct shape wins over the compatible one. Two carve-outs are stated with it. z-engine has consumers of its own and keeps the ordinary care. And this licenses changing the shape, not the invariants: the Never-Serialize Rule, fork-only sharing, prefork ordering, the arData law, publication order, lock discipline, EOWNERDEAD handling and the preemption obligations are the conditions under which this code is correct at all, not contracts that may be renegotiated for convenience. Refs #21 --- AGENTS.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 5f7a2eb..61c590e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -349,3 +349,25 @@ Scopes in use: `scheduler`, `channels`, `select`, `sync`, `context`, `poller`, ` `parallel`, `contracts`, `tests`, `tools`, `spikes`, `ci`, `docs`. Code style is PER-CS2.0, applied by php-cs-fixer. Run `composer cs:fix` rather than hand-formatting. + +## Breaking changes are allowed — prefer the correct shape + +This package and `lisachenko/php-shared-data-extension` ride development lines and have **no +external consumers**. Backwards compatibility is therefore not a constraint on either of them: +rename a method, narrow an interface, change a record layout, resize an id — whatever makes the +design right. Do not carry a deprecation cycle, do not keep a wrong method alive because something +might implement it, and do not invent an adapter to avoid touching a published shape. `LAYOUT_VERSION` +already exists to hard-fail a mismatched reader, which is the only compatibility mechanism this +family needs while it is being built. + +Two things this does **not** license. + +**`lisachenko/z-engine` is different.** It has consumers of its own, so a change there gets the +ordinary care — and, as ever, the fix for a missing capability is a named public method upstream +rather than a reach-through from here. + +**None of this applies to the invariants.** The Never-Serialize Rule, fork-only sharing, the prefork +ordering, the `arData` law, the publication order, lock discipline, `EOWNERDEAD` handling and the +preemption obligations are not API contracts — they are the conditions under which this code is +correct at all. "Breaking changes are allowed" means the *shape* is negotiable. The rules above it in +this file are not. From 824f39ed5303494596696a72748c60654e164813 Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:48:10 +0000 Subject: [PATCH 2/4] fix(tests): park on the public recv(), and assert the park actually happened testAPreemptedCoroutineThatParksIsSafeToDiscard called Channel::receive(), which is a private internal helper returning ?Delivery; the public method is recv(). The call is a fatal error whenever it executes. It survived because it usually did not execute. The coroutine only reaches that line if the shutdown drain resumes it far enough, which is timing dependent, so the bug stayed latent on 8.4 and surfaced on 8.5 in CI as an uncaught Error inside the fiber. The reason it could hide at all is the second bug: "it was drained far enough to park itself" was asserted from a flag the coroutine sets on the line *before* parking, so it only ever proved execution reached that line. A coroutine that never parked passed the assertion just as happily as one that did. Whether a park happened is only observable from the channel's own wait queue, so the test now checks pendingReceivers() once the run is over. Verified both directions: green on 8.4 and 8.5 across repeated runs, and removing the recv() call makes the new assertion report "no" and fail, which the old one did not. --- ...estAPreemptedCoroutineThatParksIsSafeToDiscard.phpt | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt b/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt index 0446c0d..cb69071 100644 --- a/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt +++ b/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt @@ -31,6 +31,11 @@ $runtime = new Runtime(preemptive: true); $runtime->run(static function (RuntimeInterface $self) use ($state): void { $silence = new Channel($self->scheduler()); + // Kept so the assertion below can look at the channel itself once the run is over. Whether + // the coroutine really parked is only observable from the channel's own wait queue: a flag + // set by the coroutine can only prove it reached the line *before* parking. + $state->channel = $silence; + Coroutine::spawn(static function () use ($state, $silence): void { $sum = 0; @@ -43,7 +48,7 @@ $runtime->run(static function (RuntimeInterface $self) use ($state): void { // Nobody ever sends here. The drain resumes this coroutine out of the preemption // callback, it parks itself on the channel, and from there it is ordinary debris that // main returning is entitled to drop. - $silence->receive(); + $silence->recv(); $state->finished = true; }); @@ -53,7 +58,8 @@ $runtime->run(static function (RuntimeInterface $self) use ($state): void { echo 'the loop was preempted: ', ($runtime->preemptor()?->preemptions() ?? 0) >= 1 ? 'yes' : 'no', PHP_EOL; -echo 'it was drained far enough to park itself: ', $state->parked ? 'yes' : 'no', PHP_EOL; +echo 'it was drained far enough to park itself: ', + $state->parked && $state->channel->pendingReceivers() === 1 ? 'yes' : 'no', PHP_EOL; echo 'it was then discarded rather than resumed: ', $state->finished ? 'no' : 'yes', PHP_EOL; ?> --EXPECT-- From 6770e072b789020f4415422a5a61892a58b70f68 Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:53:37 +0000 Subject: [PATCH 3/4] fix(tests): burn until preempted rather than for a fixed iteration count testAPreemptedCoroutineThatParksIsSafeToDiscard ran a fixed 1.5M-iteration loop and then asserted that at least one preemption had happened. How long a fixed count takes is a property of the machine, not of the runtime: on a CI runner the loop finished inside its first 10 ms slice, so the coroutine was never preempted and the test failed on a build where preemption worked correctly. The loop now runs in chunks and stops as soon as the preemption counter moves, so it is independent of machine speed. A cap bounds it, and taking the cap path leaves the counter at zero and fails the assertion - so the check still discriminates rather than becoming tautological: a build where preemption never fires fails quickly instead of spinning. Green across repeated runs on 8.4 and 8.5 with their matching z-engine lines. --- ...ptedCoroutineThatParksIsSafeToDiscard.phpt | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt b/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt index cb69071..e3edeb4 100644 --- a/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt +++ b/tests/Functional/testAPreemptedCoroutineThatParksIsSafeToDiscard.phpt @@ -16,7 +16,11 @@ use Lisachenko\NativePhpCoroutines\RuntimeInterface; include __DIR__ . '/../../vendor/autoload.php'; -const ITERATIONS = 1_500_000; +// Roughly half a 10 ms slice on the machines this has been measured on, so the preemption +// counter is re-read often enough to stop promptly without the loop itself being interrupted +// by the check. The cap bounds a build where preemption never fires: the test must fail, not spin. +const CHUNK = 250_000; +const MAX_ITERATIONS = 100_000_000; $state = new stdClass(); $state->parked = false; @@ -36,11 +40,19 @@ $runtime->run(static function (RuntimeInterface $self) use ($state): void { // set by the coroutine can only prove it reached the line *before* parking. $state->channel = $silence; - Coroutine::spawn(static function () use ($state, $silence): void { - $sum = 0; - - for ($index = 0; $index < ITERATIONS; $index++) { - $sum += $index % 7; + Coroutine::spawn(static function () use ($state, $silence, $self): void { + $sum = 0; + $index = 0; + + // Burn CPU until the scheduler has actually taken a slice back, rather than for a fixed + // number of iterations. How long a fixed count takes is a property of the machine: 1.5M + // iterations finished inside the first 10 ms slice on a CI runner, so the coroutine was + // never preempted and the test failed on a build where preemption worked perfectly. + while (($self->preemptor()?->preemptions() ?? 0) < 1 && $index < MAX_ITERATIONS) { + for ($chunk = 0; $chunk < CHUNK; $chunk++) { + $sum += $index % 7; + $index++; + } } $state->parked = true; From 7caed39955aec3c04ebaab5d816cb8bc3c2998bc Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:09:37 +0000 Subject: [PATCH 4/4] docs: state the same-address requirement instead of "sharing is fork-only" "Sharing is fork-only" described the mechanism this runtime happens to use and presented it as a property of the design. It is not one. Memory can be placed at the same virtual address in a process that was never forked from this one - shm_open plus mmap(MAP_FIXED), userfaultfd and others - so the claim foreclosed legitimate designs, and it was listed among the invariants, which made it look non-negotiable. What is actually required is narrower and worth saying precisely: every participant must see the arena at the same virtual address, and must agree on the engine pointers baked into shared structs - class entries and std_object_handlers. Under the Never-Serialize Rule an address is the value, so a shared object only means the same thing in another process when both hold. Fork gets both for free, which is why the implementation uses it, but that is the mechanism rather than the rule. The prefork ordering is unaffected and still load-bearing: a fork copies only what already exists, so shared state has to be built before it. Refs #22 --- AGENTS.md | 14 ++++++++++---- README.md | 6 ++++-- src/Runtime.php | 3 ++- src/RuntimeInterface.php | 6 ++++-- 4 files changed, 20 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 61c590e..1e6d899 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,9 +140,15 @@ The mechanism was selected by experiment, and the negative result is the load-be ## Fork, layers and locks -- **Sharing is fork-only.** Shared objects are valid because children inherit an identical address - layout. There is no attach-by-key across unrelated processes, and adding one would mean giving up - addresses as values — that is, giving up the Never-Serialize Rule. +- **Every participant must see the arena at the same virtual address, and must agree on the engine + pointers baked into shared structs** — class entries, `std_object_handlers`. Under the + Never-Serialize Rule an address *is* the value, so a shared object only means the same thing in + another process when both hold. This is the requirement; it is not a statement about how you get + there. The implementation gets both for free by forking — a child inherits its parent's mappings + and its loaded classes — but `shm_open` plus `mmap(MAP_FIXED)`, `userfaultfd` and other mechanisms + can put the same region at the same address in a process that was never forked from this one. A + design that establishes the same two guarantees another way is legitimate. What may not be given + up is the guarantees. - **Prefork ordering is load-bearing**: the arena and the shared roots are created **before** the fork; fibers are created **after** it. A fiber that exists across the fork barrier is a stack the child now owns a copy of, and a shared root created after it is not shared at all. @@ -366,7 +372,7 @@ Two things this does **not** license. ordinary care — and, as ever, the fix for a missing capability is a named public method upstream rather than a reach-through from here. -**None of this applies to the invariants.** The Never-Serialize Rule, fork-only sharing, the prefork +**None of this applies to the invariants.** The Never-Serialize Rule, the same-address requirement, the prefork ordering, the `arData` law, the publication order, lock discipline, `EOWNERDEAD` handling and the preemption obligations are not API contracts — they are the conditions under which this code is correct at all. "Breaking changes are allowed" means the *shape* is negotiable. The rules above it in diff --git a/README.md b/README.md index 1a6e5d3..d9761b1 100644 --- a/README.md +++ b/README.md @@ -475,8 +475,10 @@ z-engine requires it, and z-engine is a hard dependency of this package. `finally` blocks do not run — exactly as a goroutine's deferred calls do not run when `main` returns. - **An uncaught throwable is a panic**: it ends the run and comes back out of `Runtime::run()`. -- **Sharing is fork-only.** Shared objects are valid because children inherit an identical address - layout; there is no attach-by-key across unrelated processes. +- **Every participant must see the arena at the same address**, and must agree on the engine pointers + inside shared structs (class entries, object handlers) — an address is the value, so both have to + hold for a shared object to mean the same thing twice. Forked workers get both for free, which is + how the runtime works today; it is a requirement, not a restriction to forking forever. - **Plain arrays are not shareable.** Use `SharedArray`; a plain array grows into the private heap of whichever process filled it. Closures are shareable only by **pre-fork registration** (`registerSharedClosure()`); work created after the fork travels as a `Task`. Anything else throws diff --git a/src/Runtime.php b/src/Runtime.php index 824c629..4a80896 100644 --- a/src/Runtime.php +++ b/src/Runtime.php @@ -32,7 +32,8 @@ * * # The order everything is created in is the design * - * Fork is what makes sharing sound, and fork only copies what already exists: + * Workers must see the arena at the same address as the parent, and a fork only copies what already + * exists — so anything shared has to be built before it: * * 1. **the arena, the wake registry and the result slots** — in this constructor, so they exist * before anything else and every worker inherits them at the same address; diff --git a/src/RuntimeInterface.php b/src/RuntimeInterface.php index bbd6e6f..ce6c9b3 100644 --- a/src/RuntimeInterface.php +++ b/src/RuntimeInterface.php @@ -27,8 +27,10 @@ interface RuntimeInterface /** * Declare a named shared root, to be created **before** the workers fork. * - * Fork is what makes sharing sound: children inherit an identical address layout, so a root - * created before the fork is valid at the same address in every process. Declaring a root after + * A root is addressed, not named, once it is in the arena, so it is only usable by a process + * that sees it at the same virtual address. Forking is how this runtime arranges that today: a + * child inherits the parent's mappings, so a root created before the fork is at the same address + * everywhere, and one created afterwards exists in a single process. Declaring a root after * {@see self::run()} has forked is therefore an error, not a late binding. * * @param class-string $class Shared type to instantiate, e.g. SharedArray or SharedChannel.