Skip to content

Commit 4b25345

Browse files
committed
fix(vnext): align catalog subscription cleanup contract
1 parent cd386bb commit 4b25345

6 files changed

Lines changed: 105 additions & 142 deletions

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -514,17 +514,19 @@ or sessions. Retirement clears the cell before external cleanup, so a provider
514514
that retains an old callback retains only an inert bounded object. Synchronous
515515
callbacks are decoded into frozen epochs while `subscribe` is running; raw
516516
payloads and premature audience snapshots are not buffered. After the returned
517-
disposable is validated, the buffered epochs are staged together in FIFO
517+
cleanup closure is validated, the buffered epochs are staged together in FIFO
518518
order. Each accepted baseline or advance snapshots active membership and
519519
installs the epoch as one serialized step with no external call between those
520520
operations. A member joined during one event's dispatch therefore
521521
participates in a later pending event, but not the event already committed.
522-
A thrown subscription or malformed returned disposable discards that buffer
522+
A thrown subscription or malformed returned cleanup value discards that buffer
523523
and disables automatic invalidation for the live scope; explicit search
524524
remains available.
525-
The returned disposable is itself untrusted: the service captures one own
526-
enumerable data `dispose` closure, calls it with `this === undefined` at most
527-
once, and isolates malformed values and thrown cleanup. Dropping the last owner
525+
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 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
528530
removes the complete scope incarnation before cleanup. A later join creates a
529531
new unobserved incarnation and may attempt a fresh subscription.
530532

@@ -574,9 +576,9 @@ installed before preparation remains observed; if its submitting membership is
574576
retired during preparation, its response settles as retired while other
575577
successfully prepared members still dispatch. Coordinator disposal takes
576578
precedence over that retired evidence. The outermost cleanup barrier admits and
577-
invokes at most 1,024 captured disposers. If reentrant work attempts to admit a
579+
invokes at most 1,024 captured cleanups. If reentrant work attempts to admit a
578580
1,025th cleanup, the coordinator quarantines itself immediately: all cells and
579-
memberships become inert and all not-yet-invoked disposer references are
581+
memberships become inert and all not-yet-invoked cleanup references are
580582
cleared without calling more provider code. Cleanup is never continued from a
581583
timer.
582584

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

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,8 @@ function createFixture(memberCount: number): CoordinatorFixture {
131131
) => {
132132
subscriptionCounts.installed += 1;
133133
invalidationListener = listener;
134-
return {
135-
dispose: (): void => {
136-
subscriptionCounts.disposed += 1;
137-
},
134+
return (): void => {
135+
subscriptionCounts.disposed += 1;
138136
};
139137
},
140138
});

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

Lines changed: 68 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -143,10 +143,8 @@ function listenerProvider(): {
143143
) => {
144144
harness.scopes.push(scope);
145145
harness.listeners.push(listener);
146-
return {
147-
dispose: () => {
148-
harness.cleanupCalls += 1;
149-
},
146+
return () => {
147+
harness.cleanupCalls += 1;
150148
};
151149
},
152150
};
@@ -1093,11 +1091,11 @@ describe("subscription invalidation ordering and isolation", () => {
10931091
const events: string[] = [];
10941092
let disposeDuringPrepare = false;
10951093
let membership: SqlCatalogScopeMembership;
1096-
const owner = coordinator(() => ({
1097-
dispose: () => {
1094+
const owner = coordinator(
1095+
() => () => {
10981096
events.push("cleanup");
10991097
},
1100-
}));
1098+
);
11011099
membership = active(owner, "scope", {
11021100
prepareCatalogChange: () => {
11031101
if (disposeDuringPrepare) {
@@ -1156,7 +1154,7 @@ describe("hostile subscription lifecycle", () => {
11561154
let listenerCalls = 0;
11571155
const owner = coordinator((_scope, notify) => {
11581156
notify(invalidation(1));
1159-
return { dispose: () => {} };
1157+
return () => {};
11601158
});
11611159
const membership = prepared(owner, "scope", {
11621160
prepareCatalogChange: () => () => {
@@ -1175,7 +1173,7 @@ describe("hostile subscription lifecycle", () => {
11751173
const owner = coordinator((_scope, notify) => {
11761174
notify(invalidation(1));
11771175
expect(second.activate()).toEqual({ status: "active" });
1178-
return { dispose: () => {} };
1176+
return () => {};
11791177
});
11801178
const first = prepared(owner, "scope", firstCounter.target);
11811179
second = prepared(owner, "scope", secondCounter.target);
@@ -1200,7 +1198,7 @@ describe("hostile subscription lifecycle", () => {
12001198
const owner = coordinator((_scope, notify) => {
12011199
notify(invalidation(1));
12021200
notify(invalidation(2));
1203-
return { dispose: () => {} };
1201+
return () => {};
12041202
});
12051203
const first = prepared(owner, "scope", {
12061204
prepareCatalogChange: () => {
@@ -1231,7 +1229,7 @@ describe("hostile subscription lifecycle", () => {
12311229
for (const subscribe of [
12321230
(_scope: string, notify: RawInvalidationListener) => {
12331231
notify({ epoch: { generation: -1, token: "bad" } });
1234-
return { dispose: () => {} };
1232+
return () => {};
12351233
},
12361234
(_scope: string, notify: RawInvalidationListener) => {
12371235
notify(invalidation(1));
@@ -1253,14 +1251,15 @@ describe("hostile subscription lifecycle", () => {
12531251
expect(listenerCalls).toBe(0);
12541252
});
12551253

1256-
it("captures one own-data disposer, invokes it this-free once, and rejects hostile forms without getters", () => {
1254+
it("captures one cleanup closure, invokes it this-free once, and rejects non-functions without getters", () => {
12571255
const receivers: unknown[] = [];
12581256
let getterCalls = 0;
1259-
const validOwner = coordinator(() => ({
1260-
dispose(this: unknown) {
1261-
receivers.push(this);
1262-
},
1263-
}));
1257+
const validOwner = coordinator(
1258+
() =>
1259+
function (this: unknown) {
1260+
receivers.push(this);
1261+
},
1262+
);
12641263
const membership = active(validOwner, "scope");
12651264
membership.dispose();
12661265
membership.dispose();
@@ -1280,7 +1279,7 @@ describe("hostile subscription lifecycle", () => {
12801279
enumerable: false,
12811280
value: () => {},
12821281
});
1283-
const invalidDisposers: unknown[] = [
1282+
const invalidCleanupValues: unknown[] = [
12841283
{},
12851284
Object.create({ dispose() {} }),
12861285
nonEnumerable,
@@ -1299,8 +1298,8 @@ describe("hostile subscription lifecycle", () => {
12991298
},
13001299
},
13011300
];
1302-
for (const disposable of invalidDisposers) {
1303-
const owner = coordinator(() => disposable);
1301+
for (const cleanupValue of invalidCleanupValues) {
1302+
const owner = coordinator(() => cleanupValue);
13041303
expect(active(owner, "scope").captureEpoch().status).toBe(
13051304
"captured",
13061305
);
@@ -1312,10 +1311,8 @@ describe("hostile subscription lifecycle", () => {
13121311
let retained: RawInvalidationListener | undefined;
13131312
const owner = coordinator((_scope, notify) => {
13141313
retained = notify;
1315-
return {
1316-
dispose: () => {
1317-
throw new Error("cleanup failed");
1318-
},
1314+
return () => {
1315+
throw new Error("cleanup failed");
13191316
};
13201317
});
13211318
const target = counterTarget();
@@ -1342,7 +1339,7 @@ describe("hostile subscription lifecycle", () => {
13421339
attempts += 1;
13431340
listeners.push(notify);
13441341
if (attempts === 1) return null;
1345-
return { dispose: () => {} };
1342+
return () => {};
13461343
});
13471344
const first = active(owner, "scope");
13481345
const second = active(owner, "scope");
@@ -1364,12 +1361,10 @@ describe("hostile subscription lifecycle", () => {
13641361
let owner: SqlCatalogEpochCoordinator;
13651362
const provider = (_scope: string, notify: RawInvalidationListener) => {
13661363
listeners.push(notify);
1367-
return {
1368-
dispose: () => {
1369-
if (memberships.length === 1) {
1370-
memberships.push(active(owner, "scope"));
1371-
}
1372-
},
1364+
return () => {
1365+
if (memberships.length === 1) {
1366+
memberships.push(active(owner, "scope"));
1367+
}
13731368
};
13741369
};
13751370
owner = coordinator(provider);
@@ -1390,7 +1385,7 @@ describe("hostile subscription lifecycle", () => {
13901385
let membership: SqlCatalogScopeMembership;
13911386
const memberOwner = coordinator((_scope, notify) => {
13921387
notify(invalidation(1));
1393-
return { dispose: () => {} };
1388+
return () => {};
13941389
});
13951390
membership = prepared(memberOwner, "scope", {
13961391
prepareCatalogChange: () => () => membership.dispose(),
@@ -1403,7 +1398,7 @@ describe("hostile subscription lifecycle", () => {
14031398
let service: SqlCatalogEpochCoordinator;
14041399
service = coordinator((_scope, notify) => {
14051400
notify(invalidation(1));
1406-
return { dispose: () => {} };
1401+
return () => {};
14071402
});
14081403
const serviceMembership = prepared(service, "scope", {
14091404
prepareCatalogChange: () => () => service.dispose(),
@@ -1419,10 +1414,8 @@ describe("hostile subscription lifecycle", () => {
14191414
let cleanupCalls = 0;
14201415
const owner = coordinator((_scope, listener) => {
14211416
notify = listener;
1422-
return {
1423-
dispose: () => {
1424-
cleanupCalls += 1;
1425-
},
1417+
return () => {
1418+
cleanupCalls += 1;
14261419
};
14271420
});
14281421
const membership = active(owner, "scope");
@@ -1448,10 +1441,8 @@ describe("hostile subscription lifecycle", () => {
14481441
) {
14491442
listener({ malformed: index });
14501443
}
1451-
return {
1452-
dispose: () => {
1453-
installCleanup += 1;
1454-
},
1444+
return () => {
1445+
installCleanup += 1;
14551446
};
14561447
});
14571448
active(installOwner, "scope");
@@ -1462,7 +1453,7 @@ describe("hostile subscription lifecycle", () => {
14621453
let notify: RawInvalidationListener | undefined;
14631454
const owner = coordinator((_scope, listener) => {
14641455
notify = listener;
1465-
return { dispose: () => {} };
1456+
return () => {};
14661457
});
14671458
const membership = active(owner, "scope");
14681459
for (
@@ -1484,10 +1475,8 @@ describe("hostile subscription lifecycle", () => {
14841475
let cleanupCalls = 0;
14851476
const owner = coordinator((_scope, listener) => {
14861477
notify = listener;
1487-
return {
1488-
dispose: () => {
1489-
cleanupCalls += 1;
1490-
},
1478+
return () => {
1479+
cleanupCalls += 1;
14911480
};
14921481
});
14931482
const membership = active(owner, "scope");
@@ -1534,14 +1523,12 @@ describe("service disposal and bounded command draining", () => {
15341523
notify({ malformed: index });
15351524
}
15361525
}
1537-
return {
1538-
dispose: () => {
1539-
if (scope === "driver") {
1540-
driverCleanupCalls += 1;
1541-
} else {
1542-
overloadedCleanupCalls += 1;
1543-
}
1544-
},
1526+
return () => {
1527+
if (scope === "driver") {
1528+
driverCleanupCalls += 1;
1529+
} else {
1530+
overloadedCleanupCalls += 1;
1531+
}
15451532
};
15461533
});
15471534
const driver = active(owner, "driver", {
@@ -1581,7 +1568,7 @@ describe("service disposal and bounded command draining", () => {
15811568
const listeners = new Map<string, RawInvalidationListener>();
15821569
let cleanupCalls = 0;
15831570
let cleanupDepth = 0;
1584-
let lateDisposerCalls = 0;
1571+
let lateCleanupCalls = 0;
15851572
let maximumCleanupDepth = 0;
15861573
let sentinelCleanupCalls = 0;
15871574
let subscriptionCalls = 0;
@@ -1601,29 +1588,27 @@ describe("service disposal and bounded command draining", () => {
16011588
notify({ malformed: index });
16021589
}
16031590
}
1604-
return {
1605-
dispose: () => {
1606-
if (installsAfterCleanupLimit) {
1607-
lateDisposerCalls += 1;
1608-
return;
1609-
}
1610-
cleanupDepth += 1;
1611-
maximumCleanupDepth = Math.max(
1612-
maximumCleanupDepth,
1613-
cleanupDepth,
1591+
return () => {
1592+
if (installsAfterCleanupLimit) {
1593+
lateCleanupCalls += 1;
1594+
return;
1595+
}
1596+
cleanupDepth += 1;
1597+
maximumCleanupDepth = Math.max(
1598+
maximumCleanupDepth,
1599+
cleanupDepth,
1600+
);
1601+
cleanupCalls += 1;
1602+
if (scope === "sentinel") {
1603+
sentinelCleanupCalls += 1;
1604+
} else {
1605+
const next = active(
1606+
owner,
1607+
`cleanup-chain-${subscriptionCalls}`,
16141608
);
1615-
cleanupCalls += 1;
1616-
if (scope === "sentinel") {
1617-
sentinelCleanupCalls += 1;
1618-
} else {
1619-
const next = active(
1620-
owner,
1621-
`cleanup-chain-${subscriptionCalls}`,
1622-
);
1623-
next.dispose();
1624-
}
1625-
cleanupDepth -= 1;
1626-
},
1609+
next.dispose();
1610+
}
1611+
cleanupDepth -= 1;
16271612
};
16281613
});
16291614
const sentinelTarget = counterTarget();
@@ -1644,7 +1629,7 @@ describe("service disposal and bounded command draining", () => {
16441629
MAX_CATALOG_CLEANUPS_PER_BARRIER,
16451630
);
16461631
expect(maximumCleanupDepth).toBe(1);
1647-
expect(lateDisposerCalls).toBe(0);
1632+
expect(lateCleanupCalls).toBe(0);
16481633
expect(sentinelCleanupCalls).toBe(0);
16491634
expect(sentinel.captureEpoch()).toEqual({
16501635
reason: "disposed",
@@ -1667,7 +1652,7 @@ describe("service disposal and bounded command draining", () => {
16671652
expect(cleanupCalls).toBe(
16681653
MAX_CATALOG_CLEANUPS_PER_BARRIER,
16691654
);
1670-
expect(lateDisposerCalls).toBe(0);
1655+
expect(lateCleanupCalls).toBe(0);
16711656
expect(sentinelCleanupCalls).toBe(0);
16721657
});
16731658

@@ -1734,11 +1719,9 @@ describe("service disposal and bounded command draining", () => {
17341719
);
17351720
const owner = coordinator((_scope, listener) => {
17361721
retained = listener;
1737-
return {
1738-
dispose: () => {
1739-
cleanupCalls += 1;
1740-
retained?.(hostilePayload);
1741-
},
1722+
return () => {
1723+
cleanupCalls += 1;
1724+
retained?.(hostilePayload);
17421725
};
17431726
});
17441727
const target = counterTarget();
@@ -1940,9 +1923,7 @@ describe("service disposal and bounded command draining", () => {
19401923
const cleanupScopes: string[] = [];
19411924
const owner = coordinator((scope, notify) => {
19421925
listeners.set(scope, notify);
1943-
return {
1944-
dispose: () => cleanupScopes.push(scope),
1945-
};
1926+
return () => cleanupScopes.push(scope);
19461927
});
19471928
const stormMemberships = Array.from({ length: 5 }, (_, index) =>
19481929
active(owner, `storm-${index}`),

0 commit comments

Comments
 (0)