Skip to content

Commit 5ad22fd

Browse files
authored
feat(vnext): prepare catalog epoch transitions (#194)
Part of #169. Follows #193. - Add a package-private synchronous epoch-transition hook so shared catalog work can retire before session revision listeners observe an accepted epoch. - Preserve deterministic prepare/commit/dispatch ordering for provider invalidations and higher response epochs. - Tighten provider cleanup and transition dispatch to an exact `undefined` contract, with fail-closed quarantine for invalid returns. - Dispose before inspecting invalid thenables, closing the hostile getter reentrancy window identified after #193 merged. - Keep the no-hook and null-transition paths allocation-free and add a configured-hook storm benchmark. - Extend ADR 0005, type fixtures, lifecycle tests, and adversarial Promise/thenable coverage. <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Adds a package-private, synchronous epoch-transition hook that runs after epoch install to retire catalog work before session listeners. Enforces exact-return contracts and fail-closed handling to harden reentrancy, cleanup, and poisoned-thenable edge cases; supports #169. - **New Features** - Added `SqlCatalogEpochTransitionTarget` that may return a dispatch closure or `null`; dispatch must return exactly `undefined` (sync only). - Preserved strict ordering: transition-prepare → revision-prepare → producer settle → transition-dispatch → revision-dispatch; reentrant dispatch queues behind current dispatch. - Snapshots the revision audience before transition; producers/aborts see prepared revisions; listeners see already-retired work. - Rejects non-function transition targets during coordinator creation (`reason: "invalid-transition-target"`); any thrown preparation, thrown dispatch, or non-`undefined` return disposes fail-closed and discards response decisions. - Drains detached Promise/thenable results for cleanup and transition returns after disposal, avoiding hostile getter reentrancy; no-hook and `null`-dispatch paths allocate nothing. Updated ADR 0005, added a configured-hook storm benchmark, and expanded adversarial tests. - **Migration** - Update `SqlCatalogSubscriptionCleanup` to be synchronous and return `undefined` only (no Promises or other return values). <sup>Written for commit 9d7c9c3. Summary will update on new commits.</sup> <a href="https://cubic.dev/pr/marimo-team/codemirror-sql/pull/194?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
1 parent ffa8229 commit 5ad22fd

6 files changed

Lines changed: 778 additions & 22 deletions

docs/adr/0005-parser-independent-relation-completion.md

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,28 @@ A higher accepted invalidation or response atomically installs the new epoch,
463463
clears older scope cache entries, supersedes affected work, and then advances
464464
each subscribed session revision exactly once.
465465

466+
The epoch coordinator accepts one optional package-owned, receiver-free
467+
transition-preparation closure. For an accepted invalidation baseline or
468+
advance, and for a higher response, it snapshots the active revision audience,
469+
installs the epoch and notification sequence, and then calls that closure with
470+
a copied scope and the already-frozen epoch. The closure synchronously detaches
471+
incompatible work, cache entries, and epoch-specific gates and may return one
472+
receiver-free dispatch closure. The no-hook and no-dispatch paths allocate no
473+
transition wrapper state. Only after transition and session preparation
474+
has finished does the coordinator settle the producing response, invoke the
475+
transition dispatch, and dispatch session listeners. Provider aborts and work
476+
settlement therefore observe every prepared session revision, while listener
477+
code observes already-retired catalog work. Reentrant transition dispatch
478+
commands remain behind the current revision dispatch in the epoch FIFO.
479+
A missing or `null` dispatch is valid. A thrown transition preparation or
480+
non-function result is an internal contract failure and disposes the
481+
coordinator fail-closed. Dispatch is synchronously exact-return: it must return
482+
`undefined`. A throw or any other return value disposes the coordinator after
483+
state preparation and suppresses later revision listeners; an accidental
484+
Promise result receives best-effort detached rejection draining. Coordinator
485+
disposal revokes the preparation closure before external cleanup. The hook
486+
remains package-private and is not a provider or session extension point.
487+
466488
A search that discovers a higher epoch supersedes itself instead of publishing
467489
against its older captured revision. Pages and cache entries from different
468490
epochs are never merged.
@@ -523,13 +545,21 @@ A thrown subscription or malformed returned cleanup value discards that buffer
523545
and disables automatic invalidation for the live scope; explicit search
524546
remains available.
525547
The returned cleanup closure is itself untrusted: the service captures only a
526-
function, calls it with `this === undefined` at most once, and isolates
527-
malformed values, thrown cleanup, and rejected or hostile thenable results.
528-
Detached cleanup settlement retains no coordinator state. A closure avoids a
529-
structural TypeScript contract that would accept class instances or inherited
530-
methods which the hostile runtime boundary could not safely validate. Dropping
531-
the last owner removes the complete scope incarnation before cleanup. A later
532-
join creates a new unobserved incarnation and may attempt a fresh subscription.
548+
function and calls it with `this === undefined` at most once. Its exact
549+
synchronous contract returns `undefined`; TypeScript therefore rejects async
550+
cleanup functions rather than silently accepting them through `void`
551+
assignability. A throw or any other return value quarantines the coordinator
552+
after the subscription is inert. Accidental ordinary Promise or thenable
553+
returns receive best-effort detached rejection draining, but a same-realm
554+
callback can always create a poisoned or independent unhandled rejection that
555+
no JavaScript library can retroactively contain. Legitimate asynchronous
556+
teardown must consume or report its own failure before the cleanup closure
557+
returns `undefined`. The valid `undefined` path returns without Promise
558+
allocation or a microtask. A closure avoids a structural TypeScript
559+
contract that would accept class instances or inherited methods which the
560+
hostile runtime boundary could not safely validate. Dropping the last owner
561+
removes the complete scope incarnation before cleanup. A later join creates a
562+
new unobserved incarnation and may attempt a fresh subscription.
533563

534564
Malformed, duplicate, stale, or token-conflicting invalidations do not mutate
535565
state and do not tear down an otherwise valid subscription. Every raw callback

src/vnext/__tests__/relation-catalog-epoch-coordinator.bench.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from "../relation-catalog-epoch-coordinator.js";
99
import type {
1010
SqlCatalogEpochCoordinator,
11+
SqlCatalogEpochTransitionTarget,
1112
SqlCatalogResponseEpochDecision,
1213
SqlCatalogResponseEpochSubmissionResult,
1314
SqlCatalogRevisionTarget,
@@ -110,7 +111,10 @@ function requireDecision(
110111
return decision;
111112
}
112113

113-
function createFixture(memberCount: number): CoordinatorFixture {
114+
function createFixture(
115+
memberCount: number,
116+
prepareEpochTransition?: SqlCatalogEpochTransitionTarget,
117+
): CoordinatorFixture {
114118
let invalidationListener:
115119
| ((this: void, event: unknown) => void)
116120
| null = null;
@@ -141,6 +145,7 @@ function createFixture(memberCount: number): CoordinatorFixture {
141145
}
142146
const created = createSqlCatalogEpochCoordinator(
143147
captured.value,
148+
prepareEpochTransition,
144149
);
145150
if (created.status !== "created") {
146151
return benchmarkFailure("coordinator creation was unavailable");
@@ -416,6 +421,32 @@ describe("relation catalog epoch coordinator", () => {
416421
},
417422
);
418423

424+
bench(
425+
"process a bounded 256-event storm with a null transition dispatch",
426+
() => {
427+
let transitions = 0;
428+
const fixture = createFixture(1, () => {
429+
transitions += 1;
430+
return null;
431+
});
432+
for (
433+
let generation = 1;
434+
generation <= MAX_CATALOG_CALLBACKS_PER_RESET_WINDOW;
435+
generation += 1
436+
) {
437+
fixture.emitInvalidation(generation);
438+
}
439+
if (
440+
transitions !== MAX_CATALOG_CALLBACKS_PER_RESET_WINDOW
441+
) {
442+
benchmarkFailure(
443+
"configured transition hook lost an accepted epoch",
444+
);
445+
}
446+
fixture.dispose();
447+
},
448+
);
449+
419450
bench("fan out a bounded 256-event storm to 256 members", () => {
420451
const fixture = createFixture(256);
421452
for (

0 commit comments

Comments
 (0)