From cfd0e49e88566e802b5d97a23d09866fa236f06a Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sun, 16 Aug 2026 08:33:09 +0000 Subject: [PATCH 1/2] fix(parallel): report a detail-less panic as unavailable, never as another task's MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The close-out of #19, Option 1. The cardinality fix itself landed with the substrate's per-instance keying (#25 here, lisachenko/php-shared-data-extension#23): each panic is its own SharedError instance graph whose address rides in the panicking task's own slot, so two concurrent panics can no longer share one error entry. What remained was pinning the acceptance criteria and making the detail-unavailable path tell the truth. - The fallback for a panic slot whose payload does not attach as a SharedError was a WorkerCrashedException — but the worker settled the slot, so it is demonstrably alive, and a waiter catching ParallelTaskException never saw the panic at all. It is now a ParallelTaskException that states the detail is unavailable, with no fabricated class and no borrowed trace — the same shape the no-arena supervisor path already uses for a detail-less PANIC record. - testEachPanicKeepsItsOwnSharedError now pins all three fields of the first acceptance criterion: the two concurrent panics differ by class, message and task frame, and each waiter asserts its exception carries its own class, its own message AND its own trace — with the other task's frame asserted absent. - New testAPanicWhoseDetailCannotBeAttachedSaysSoExplicitly pins the third criterion by settling a panic slot by hand with the address of a shared object that is not a SharedError: the waiter still sees a task panic, told explicitly that the detail is unavailable. The second criterion — no panic path serializes anything — is the existing source-scanning test, untouched and still green. Closes #19 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA --- AGENTS.md | 4 +- src/Parallel/SlotTable.php | 15 +++-- ...etailCannotBeAttachedSaysSoExplicitly.phpt | 67 +++++++++++++++++++ .../testEachPanicKeepsItsOwnSharedError.phpt | 34 +++++++--- tests/Support/shared.php | 14 ++++ 5 files changed, 120 insertions(+), 14 deletions(-) create mode 100644 tests/Functional/testAPanicWhoseDetailCannotBeAttachedSaysSoExplicitly.phpt diff --git a/AGENTS.md b/AGENTS.md index eebe713..0349c2e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -236,7 +236,9 @@ in every process. the fork, which allocates nothing per spawn. - **One `SharedError` per panic.** Each capture is its own instance graph, so two workers failing near-simultaneously each leave an error their waiter can still attach by the address its own - slot carries. + slot carries. A panic slot whose payload does not attach as a `SharedError` still surfaces as a + `ParallelTaskException` — one that says the detail is unavailable, never one that presents + another object's fields as this task's failure. - **Result slots are bump-allocated from a pre-sized table and never given back.** They are a bounded supply for the life of the arena, which is what `soak-arena-watermark.php` reports rather than assumes. diff --git a/src/Parallel/SlotTable.php b/src/Parallel/SlotTable.php index 010119d..cb1352f 100644 --- a/src/Parallel/SlotTable.php +++ b/src/Parallel/SlotTable.php @@ -13,7 +13,6 @@ namespace Lisachenko\NativePhpCoroutines\Parallel; use Lisachenko\NativePhpCoroutines\Exception\ParallelTaskException; -use Lisachenko\NativePhpCoroutines\Exception\WorkerCrashedException; use Lisachenko\NativePhpCoroutines\Parallel\Protocol\TaggedRecord; use Lisachenko\NativePhpCoroutines\SchedulerInterface; use Lisachenko\SharedData\Ipc\SharedError; @@ -219,6 +218,12 @@ public function failPendingOf(int $workerId, \Throwable $error): void * code write a per-process `properties` pointer into the shared struct, and the next sibling to * read that object segfaults. A panic handler is exactly the code most likely to reach for a * dump, which is why it is spelled out here rather than assumed. + * + * A panic slot whose payload does not attach as a {@see SharedError} still surfaces as a + * `ParallelTaskException` — the panic itself is certain, only its detail is missing — and the + * exception says the detail is unavailable rather than presenting whatever object was found as + * this task's failure. The worker is not declared dead over it: it settled the slot, so it is + * demonstrably alive. */ private function settle(ResultSlot $slot, SlotResult $result): void { @@ -237,10 +242,12 @@ private function settle(ResultSlot $slot, SlotResult $result): void $slot->complete = true; $slot->error = $error instanceof SharedError ? new ParallelTaskException($error->className, $error->message, $error->trace, $slot->workerId) - : new WorkerCrashedException( + : new ParallelTaskException( + 'Throwable', + 'its error detail is unavailable — the slot payload did not attach as a shared ' + . 'error-info object, and no other task\'s detail is presented in its place', + '', $slot->workerId, - 'the task panicked but its shared error-info object could not be attached', - [$slot->id], ); $this->wake($slot); diff --git a/tests/Functional/testAPanicWhoseDetailCannotBeAttachedSaysSoExplicitly.phpt b/tests/Functional/testAPanicWhoseDetailCannotBeAttachedSaysSoExplicitly.phpt new file mode 100644 index 0000000..8254e5d --- /dev/null +++ b/tests/Functional/testAPanicWhoseDetailCannotBeAttachedSaysSoExplicitly.phpt @@ -0,0 +1,67 @@ +--TEST-- +A panic slot whose detail does not attach as a SharedError says so, instead of presenting another object as the failure +--INI-- +ffi.enable=1 +opcache.jit=off +error_reporting=E_ALL & ~E_DEPRECATED +--FILE-- +run(static function (TaskRuntime $self) use ($runtime): void { + parallelDeadline(15.0, 'the detail-less panic reaching its waiter'); + + // A deliberate rig: slots are settled by hand here, which is machinery the task surface + // intentionally does not carry, so the arena is read off the concrete runtime. + $arena = $runtime->arena(); + if ($arena === null) { + throw new LogicException('this runtime has no arena'); + } + + $decoy = $self->persist(new SharedCounter()); + $slotId = $arena->slotTable()->allocateSlot(); + $arena->slotTable()->completePanic($slotId, $arena->addressOf($decoy)); + + try { + $self->attachResult($slotId)->await(); + + echo 'the await returned, which it must not', PHP_EOL; + } catch (ParallelTaskException $panic) { + echo 'the panic still surfaces as a task panic: yes', PHP_EOL; + echo 'it says the detail is unavailable: ', + str_contains($panic->getMessage(), 'error detail is unavailable') ? 'yes' : 'no', PHP_EOL; + echo 'no fabricated class: ', $panic->originalClass() === 'Throwable' ? 'yes' : 'no', PHP_EOL; + echo 'no borrowed trace: ', $panic->originalTrace() === '' ? 'yes' : 'no', PHP_EOL; + } +}); + +echo 'children left: ', parallelChildrenLeft(), PHP_EOL; +?> +--EXPECT-- +the panic still surfaces as a task panic: yes +it says the detail is unavailable: yes +no fabricated class: yes +no borrowed trace: yes +children left: none diff --git a/tests/Functional/testEachPanicKeepsItsOwnSharedError.phpt b/tests/Functional/testEachPanicKeepsItsOwnSharedError.phpt index a8bab69..0ee1932 100644 --- a/tests/Functional/testEachPanicKeepsItsOwnSharedError.phpt +++ b/tests/Functional/testEachPanicKeepsItsOwnSharedError.phpt @@ -1,5 +1,5 @@ --TEST-- -Two panicking workers each keep their own error, and each waiter reads its own +Two panicking workers each keep their own error: class, message and trace all belong to the right task --INI-- ffi.enable=1 opcache.jit=off @@ -12,6 +12,7 @@ declare(strict_types=1); use Lisachenko\NativePhpCoroutines\Exception\ParallelTaskException; use Lisachenko\NativePhpCoroutines\Runtime; use Lisachenko\NativePhpCoroutines\TaskRuntime; +use Lisachenko\NativePhpCoroutines\Tests\Support\PanicWithDomainErrorTask; use Lisachenko\NativePhpCoroutines\Tests\Support\PanicWithMessageTask; use Lisachenko\NativePhpCoroutines\Timer; @@ -23,8 +24,10 @@ include __DIR__ . '/../Support/shared.php'; // A captured panic used to be persisted under SharedError's class name, so the second worker's // capture superseded the first worker's registry entry — a waiter attaching the first error's -// address after that found nothing. Each capture is now its own instance graph: two workers -// failing near-simultaneously each leave an error, and each waiter reads exactly its own. +// address after that found nothing, or worse, another task's detail. Each capture is now its own +// instance graph whose address rides in the panicking task's own slot, so each waiter reads +// exactly its own failure: the two tasks here panic with different classes and messages, and each +// exception's class, message AND trace must all belong to the task that was awaited. $runtime = new Runtime(workers: 2, arenaSize: 32 << 20); $runtime->run(static function (TaskRuntime $self): void { @@ -32,17 +35,26 @@ $runtime->run(static function (TaskRuntime $self): void { throw new RuntimeException('deadline: the two panics never reached their waiters'); }); - $one = $self->spawnParallel(new PanicWithMessageTask('the first worker exploded'), 0); - $two = $self->spawnParallel(new PanicWithMessageTask('the second worker exploded'), 1); + $handles = [ + 'first' => [$self->spawnParallel(new PanicWithMessageTask('the first worker exploded'), 0), + 'RuntimeException', 'PanicWithMessageTask', 'PanicWithDomainErrorTask'], + 'second' => [$self->spawnParallel(new PanicWithDomainErrorTask('the second worker exploded'), 1), + 'DomainException', 'PanicWithDomainErrorTask', 'PanicWithMessageTask'], + ]; - foreach (['first' => $one, 'second' => $two] as $which => $handle) { + foreach ($handles as $which => [$handle, $ownClass, $ownFrame, $otherFrame]) { try { $handle->await(); echo $which, ': the await returned, which it must not', PHP_EOL; } catch (ParallelTaskException $panic) { - echo $which, ' waiter got its own panic: ', + echo $which, ' class is its own: ', + $panic->originalClass() === $ownClass ? 'yes' : 'no (' . $panic->originalClass() . ')', PHP_EOL; + echo $which, ' message is its own: ', str_contains($panic->getMessage(), "the {$which} worker exploded") ? 'yes' : 'no', PHP_EOL; + echo $which, ' trace is its own: ', + str_contains($panic->originalTrace(), $ownFrame) + && !str_contains($panic->originalTrace(), $otherFrame) ? 'yes' : 'no', PHP_EOL; } } }); @@ -50,6 +62,10 @@ $runtime->run(static function (TaskRuntime $self): void { echo 'children left: ', parallelChildrenLeft(), PHP_EOL; ?> --EXPECT-- -first waiter got its own panic: yes -second waiter got its own panic: yes +first class is its own: yes +first message is its own: yes +first trace is its own: yes +second class is its own: yes +second message is its own: yes +second trace is its own: yes children left: none diff --git a/tests/Support/shared.php b/tests/Support/shared.php index 1f893c2..6d623ee 100644 --- a/tests/Support/shared.php +++ b/tests/Support/shared.php @@ -195,6 +195,20 @@ public function run(TaskRuntime $runtime): mixed } } +/** + * Panics with a DomainException, so a concurrent panic differs from {@see PanicWithMessageTask}'s + * by class and by the task frame in its trace — not only by message. + */ +final class PanicWithDomainErrorTask implements Task +{ + public function __construct(public readonly string $message) {} + + public function run(TaskRuntime $runtime): mixed + { + throw new \DomainException($this->message); + } +} + /** Pushes a bounded number of values onto a named shared channel. */ final class SharedSendTask implements Task { From 44f41b47fb7c001244b36e8b93f2594eee0dfd06 Mon Sep 17 00:00:00 2001 From: Alexander Lisachenko <640114+lisachenko@users.noreply.github.com> Date: Sun, 16 Aug 2026 08:33:09 +0000 Subject: [PATCH 2/2] docs(parallel): drop the stale class-keyed claim from SharedArena::persist() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The docblock still described the substrate's old registry contract — class-keyed storage where a second persist of one class is an upsert — while the method body beneath it has ridden persistInstance() since the per-instance keying landed. Say what the code does: one entry per graph, named by its own root address, any number of one class live at once. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01U5QRWPfmkZME7hDjU6i5EA --- src/Parallel/SharedArena.php | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Parallel/SharedArena.php b/src/Parallel/SharedArena.php index 5042ab7..e23678e 100644 --- a/src/Parallel/SharedArena.php +++ b/src/Parallel/SharedArena.php @@ -407,10 +407,9 @@ public function recheck(): void /** * Clone an object graph into the arena and hand back the shared instance. * - * Storage is keyed by class, which is the substrate's registry contract: persisting a second - * instance of the same class is an **upsert** that supersedes the first. A design that needs - * several live graphs of one class gives them distinct classes, or puts them in a - * `SharedArray`. + * Storage is keyed per instance: the substrate registers the graph under a name minted from + * its own root address, so persisting a second instance of the same class is a second entry — + * never an upsert — and any number of graphs of one class are live at once. * * The graph is persisted `mutable: true`, so a worker's writes are visible to the family * instead of being rolled back at request end. A bare `$object->prop = …` on the result is