Skip to content

Commit 0ac451c

Browse files
committed
fix(vnext): drain catalog cleanup settlement
1 parent 4b25345 commit 0ac451c

4 files changed

Lines changed: 80 additions & 7 deletions

File tree

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -524,11 +524,12 @@ and disables automatic invalidation for the live scope; explicit search
524524
remains available.
525525
The returned cleanup closure is itself untrusted: the service captures only a
526526
function, calls it with `this === undefined` at most once, and isolates
527-
malformed values and thrown cleanup. A closure avoids a structural TypeScript
528-
contract that would accept class instances or inherited methods which the
529-
hostile runtime boundary could not safely validate. Dropping the last owner
530-
removes the complete scope incarnation before cleanup. A later join creates a
531-
new unobserved incarnation and may attempt a fresh subscription.
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.
532533

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

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1332,6 +1332,74 @@ describe("hostile subscription lifecycle", () => {
13321332
expect(failedTarget.dispatched).toBe(0);
13331333
});
13341334

1335+
it("drains rejected and hostile cleanup results after making state inert", async () => {
1336+
let asyncCleanupCalls = 0;
1337+
const rejectingOwner = coordinator(
1338+
() => async () => {
1339+
asyncCleanupCalls += 1;
1340+
throw new Error("async cleanup failed");
1341+
},
1342+
);
1343+
const rejectingMembership = active(rejectingOwner, "scope");
1344+
rejectingMembership.dispose();
1345+
rejectingMembership.dispose();
1346+
1347+
let applyCalls = 0;
1348+
let thenReads = 0;
1349+
const thenProperty = ["th", "en"].join("");
1350+
const hostileResult = Object.defineProperty({}, thenProperty, {
1351+
get() {
1352+
thenReads += 1;
1353+
throw new Error("hostile then getter");
1354+
},
1355+
});
1356+
const hostileCleanup = new Proxy(() => hostileResult, {
1357+
apply(target, receiver, argumentsList) {
1358+
applyCalls += 1;
1359+
expect(receiver).toBeUndefined();
1360+
return Reflect.apply(target, receiver, argumentsList);
1361+
},
1362+
});
1363+
const hostileOwner = coordinator(() => hostileCleanup);
1364+
const hostileMembership = active(hostileOwner, "scope");
1365+
hostileMembership.dispose();
1366+
hostileMembership.dispose();
1367+
1368+
let thenCalls = 0;
1369+
let reentrantOwner: SqlCatalogEpochCoordinator;
1370+
const reentrantResult = Object.defineProperty(
1371+
{},
1372+
thenProperty,
1373+
{
1374+
value: (resolve: () => void): void => {
1375+
thenCalls += 1;
1376+
const replacement = active(
1377+
reentrantOwner,
1378+
"replacement",
1379+
);
1380+
replacement.dispose();
1381+
resolve();
1382+
},
1383+
},
1384+
);
1385+
const reentrantCleanup = () => reentrantResult;
1386+
let reentrantSubscriptions = 0;
1387+
reentrantOwner = coordinator(() => {
1388+
reentrantSubscriptions += 1;
1389+
return reentrantSubscriptions === 1
1390+
? reentrantCleanup
1391+
: () => {};
1392+
});
1393+
active(reentrantOwner, "scope").dispose();
1394+
1395+
await Promise.resolve();
1396+
await Promise.resolve();
1397+
expect(asyncCleanupCalls).toBe(1);
1398+
expect(applyCalls).toBe(1);
1399+
expect(thenReads).toBe(1);
1400+
expect(thenCalls).toBe(1);
1401+
});
1402+
13351403
it("retries a failed subscription only in a new last-owner incarnation", () => {
13361404
const listeners: RawInvalidationListener[] = [];
13371405
let attempts = 0;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,7 @@ const ACTIVE_RESULT: SqlCatalogMembershipActivationResult =
226226
const SUBMITTED_RESULT: SqlCatalogResponseEpochSubmissionResult =
227227
Object.freeze({ status: "submitted" });
228228
const NO_PREPARE_CATALOG_CHANGE = (): null => null;
229+
const IGNORE_CLEANUP_REJECTION = (): void => {};
229230

230231
function unavailableActivation(
231232
reason: Exclude<
@@ -375,7 +376,8 @@ function cleanupSubscription(subscription: SubscriptionState): void {
375376
subscription.cleanupCalled = true;
376377
subscription.cleanup = null;
377378
try {
378-
Reflect.apply(cleanup, undefined, []);
379+
const result = Reflect.apply(cleanup, undefined, []);
380+
void Promise.resolve(result).catch(IGNORE_CLEANUP_REJECTION);
379381
} catch {
380382
// Provider cleanup is isolated after state is inert.
381383
}

src/vnext/relation-completion-types.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ export interface SqlDisposable {
1212
readonly dispose: (this: void) => void;
1313
}
1414

15-
export type SqlCatalogSubscriptionCleanup = (this: void) => void;
15+
export type SqlCatalogSubscriptionCleanup = (
16+
this: void,
17+
) => void | PromiseLike<void>;
1618

1719
export type SqlCatalogContainerRole =
1820
| "catalog"

0 commit comments

Comments
 (0)