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/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 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 {