Skip to content

Commit 1c1f8fd

Browse files
committed
fix(vnext): close failed catalog capture lifecycle
1 parent 90b3681 commit 1c1f8fd

5 files changed

Lines changed: 147 additions & 11 deletions

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,9 @@ callback can always create a poisoned or independent unhandled rejection that
561561
no JavaScript library can retroactively contain. Legitimate asynchronous
562562
teardown must consume or report its own failure before the cleanup closure
563563
returns `undefined`. The valid `undefined` path returns without Promise
564-
allocation or a microtask. A closure avoids a structural TypeScript
564+
allocation or a microtask. Detached assimilation uses module-captured Promise
565+
intrinsics, so replacing the global Promise constructor cannot disable the
566+
drain. A closure avoids a structural TypeScript
565567
contract that would accept class instances or inherited methods which the
566568
hostile runtime boundary could not safely validate. Dropping the last owner
567569
removes the complete scope incarnation before cleanup. A later join creates a
@@ -660,6 +662,8 @@ outcomes settle through a discriminated request result.
660662
Completion is latest-wins per session:
661663

662664
- a new completion request supersedes the previous request;
665+
- a new request that cannot capture an active epoch still supersedes and
666+
detaches the previous request before reporting its unavailable outcome;
663667
- same-key supersession atomically attaches the new request consumer, or
664668
retags the existing observer as that consumer, before removing old request
665669
ownership; different-key supersession revokes the old observer first;

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

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,56 @@ describe("catalog epoch coordinator construction and membership", () => {
288288
await Promise.resolve();
289289
});
290290

291+
it("drains a plain thenable with captured Promise intrinsics after the global constructor is replaced", async () => {
292+
const intrinsicPromise = Promise;
293+
let thenCalls = 0;
294+
const thenable = new Proxy(
295+
{},
296+
{
297+
get(target, property, receiver): unknown {
298+
if (property === "then") {
299+
return (
300+
_resolve: (value: unknown) => void,
301+
reject: (reason?: unknown) => void,
302+
): void => {
303+
thenCalls += 1;
304+
reject(new Error("detached plain thenable"));
305+
};
306+
}
307+
return Reflect.get(target, property, receiver);
308+
},
309+
},
310+
);
311+
function HostilePromise(): never {
312+
throw new Error("mutable global Promise was used");
313+
}
314+
expect(
315+
Reflect.set(globalThis, "Promise", HostilePromise),
316+
).toBe(true);
317+
try {
318+
const created = Reflect.apply(
319+
createSqlCatalogEpochCoordinator,
320+
undefined,
321+
[
322+
capturedProvider(),
323+
undefined,
324+
() => thenable,
325+
],
326+
);
327+
expect(created.status).toBe("created");
328+
if (created.status !== "created") {
329+
throw new Error("Expected a coordinator fixture");
330+
}
331+
const createdCoordinator = created.coordinator;
332+
expect(() => createdCoordinator.dispose()).not.toThrow();
333+
} finally {
334+
Reflect.set(globalThis, "Promise", intrinsicPromise);
335+
}
336+
await intrinsicPromise.resolve();
337+
await intrinsicPromise.resolve();
338+
expect(thenCalls).toBe(1);
339+
});
340+
291341
it("validates exact bounded well-formed scopes without raw errors", () => {
292342
const owner = coordinator();
293343
expect(

src/vnext/__tests__/relation-catalog-search-work.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1330,6 +1330,78 @@ describe("catalog search epoch authority and isolation", () => {
13301330
});
13311331
});
13321332

1333+
it("supersedes reentrant work when its retiring membership makes the next capture fail", async () => {
1334+
const listener: {
1335+
current: ((event: unknown) => void) | null;
1336+
} = { current: null };
1337+
const calls: ProviderCall[] = [];
1338+
const captured = accepted(
1339+
captureSqlRelationCatalogProvider({
1340+
id: "catalog",
1341+
search(
1342+
request: SqlCatalogSearchRequest,
1343+
signal: AbortSignal,
1344+
) {
1345+
const settlement = deferred<unknown>();
1346+
calls.push({ request, settlement, signal });
1347+
return settlement.promise;
1348+
},
1349+
subscribe(
1350+
_scope: string,
1351+
onInvalidation: (event: unknown) => void,
1352+
) {
1353+
listener.current = onInvalidation;
1354+
return () => undefined;
1355+
},
1356+
}),
1357+
);
1358+
const service = coordinator(captured);
1359+
let session: SqlCatalogSearchWorkOwner | null = null;
1360+
const reentrant: {
1361+
current: SqlCatalogSearchWorkTicket | null;
1362+
} = { current: null };
1363+
const prepared = service.prepareOwner(
1364+
"scope",
1365+
POSTGRESQL_SQL_RELATION_DIALECT,
1366+
{
1367+
prepareCatalogChange: () => {
1368+
reentrant.current =
1369+
session?.request(input("reentrant")) ?? null;
1370+
return null;
1371+
},
1372+
},
1373+
);
1374+
expect(prepared.status).toBe("prepared");
1375+
if (prepared.status !== "prepared") {
1376+
throw new Error("Expected prepared owner");
1377+
}
1378+
session = prepared.owner;
1379+
expect(session.activate()).toEqual({ status: "active" });
1380+
const initial = session.request(input("initial"));
1381+
1382+
listener.current?.({ epoch: epoch(1) });
1383+
await expect(initial.result).resolves.toEqual({
1384+
status: "superseded",
1385+
});
1386+
expect(reentrant.current).not.toBeNull();
1387+
expect(calls).toHaveLength(2);
1388+
expect(calls[0]?.signal.aborted).toBe(true);
1389+
expect(calls[1]?.signal.aborted).toBe(false);
1390+
1391+
const afterRetirement = session.request(input("after"));
1392+
await expect(afterRetirement.result).resolves.toEqual({
1393+
reason: "disposed",
1394+
status: "unavailable",
1395+
});
1396+
if (!reentrant.current) {
1397+
throw new Error("Expected reentrant request");
1398+
}
1399+
await expect(reentrant.current.result).resolves.toEqual({
1400+
status: "superseded",
1401+
});
1402+
expect(calls[1]?.signal.aborted).toBe(true);
1403+
});
1404+
13331405
it("makes a higher response self-supersede and retire other same-scope work only", async () => {
13341406
const provider = providerHarness();
13351407
const service = coordinator(provider.captured);

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ const SUBMITTED_RESULT: SqlCatalogResponseEpochSubmissionResult =
240240
Object.freeze({ status: "submitted" });
241241
const NO_PREPARE_CATALOG_CHANGE = (): null => null;
242242
const IGNORE_DETACHED_REJECTION = (): void => {};
243+
const INTRINSIC_PROMISE = Promise;
244+
const INTRINSIC_PROMISE_RESOLVE = Promise.resolve;
243245
const INTRINSIC_PROMISE_THEN = Promise.prototype.then;
244246
const FAILED_EPOCH_TRANSITION: unique symbol = Symbol(
245247
"FailedSqlCatalogEpochTransition",
@@ -392,21 +394,18 @@ function drainDetachedSettlement(result: unknown): void {
392394
return;
393395
}
394396
try {
395-
Reflect.apply(INTRINSIC_PROMISE_THEN, result, [
397+
const settlement = Reflect.apply(
398+
INTRINSIC_PROMISE_RESOLVE,
399+
INTRINSIC_PROMISE,
400+
[result],
401+
);
402+
Reflect.apply(INTRINSIC_PROMISE_THEN, settlement, [
396403
undefined,
397404
IGNORE_DETACHED_REJECTION,
398405
]);
399-
return;
400406
} catch {
401-
// Non-native thenables are assimilated through a fresh wrapper.
407+
// The detached value is hostile and cannot be observed safely.
402408
}
403-
const settlement = new Promise<unknown>((resolve) => {
404-
resolve(result);
405-
});
406-
Reflect.apply(INTRINSIC_PROMISE_THEN, settlement, [
407-
undefined,
408-
IGNORE_DETACHED_REJECTION,
409-
]);
410409
}
411410

412411
function cleanupSubscription(

src/vnext/relation-catalog-search-work.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1463,6 +1463,17 @@ function requestWork(
14631463
return makeImmediateTicket(SUPERSEDED_OUTCOME);
14641464
}
14651465
if (captured.status !== "captured") {
1466+
const pending = effects();
1467+
const previous = owner.current;
1468+
if (previous) {
1469+
detachConsumerInto(
1470+
state,
1471+
previous,
1472+
SUPERSEDED_OUTCOME,
1473+
pending,
1474+
);
1475+
}
1476+
runEffects(state, pending);
14661477
return makeImmediateTicket(
14671478
unavailableOutcome(
14681479
captured.reason === "inactive"

0 commit comments

Comments
 (0)