Skip to content

Commit fe600b5

Browse files
fix(shared): keyed locks keep working after the scope that made them
ProviderMaintenanceRunner is built per WebSocket connection inside an Effect.provide whose scope closes before any request is served. The RcMap-backed lock closed with it, so provider updates were interrupted after being marked queued and stayed queued. KeyedLock is now a refcounted Map of semaphores, as KeyedSerialExecutor was before, so it needs no scope. The maintenance runner tests go back to main's harness, which builds the runner the way ws.ts does. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
1 parent 075fcc4 commit fe600b5

4 files changed

Lines changed: 63 additions & 47 deletions

File tree

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import * as KeyedLock from "@t3tools/shared/KeyedLock";
22
import type * as Effect from "effect/Effect";
3-
import type * as Scope from "effect/Scope";
43

54
export interface KeyedSerialExecutor<Key> {
65
readonly withLock: <A, E, R>(key: Key, effect: Effect.Effect<A, E, R>) => Effect.Effect<A, E, R>;
@@ -10,8 +9,5 @@ export interface KeyedSerialExecutor<Key> {
109
* Serializes work that targets the same domain identity without coupling
1110
* unrelated identities to a process-wide mutex.
1211
*/
13-
export const makeKeyedSerialExecutor = <Key>(): Effect.Effect<
14-
KeyedSerialExecutor<Key>,
15-
never,
16-
Scope.Scope
17-
> => KeyedLock.make<Key>();
12+
export const makeKeyedSerialExecutor = <Key>(): Effect.Effect<KeyedSerialExecutor<Key>> =>
13+
KeyedLock.make<Key>();

‎apps/server/src/provider/providerMaintenanceRunner.test.ts‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {
77
} from "@t3tools/contracts";
88
import { ServerProviderUpdateError } from "@t3tools/contracts";
99
import * as Cause from "effect/Cause";
10-
import * as Context from "effect/Context";
1110
import * as Effect from "effect/Effect";
1211
import * as Exit from "effect/Exit";
1312
import * as Fiber from "effect/Fiber";
@@ -224,27 +223,24 @@ const makeTestRunner = (
224223
})),
225224
},
226225
) =>
227-
// Built in the test's scope: the runner must stay open while the test uses it.
228-
Layer.build(
229-
ProviderMaintenanceRunner.layer.pipe(
230-
Layer.provide(
231-
Layer.mergeAll(
232-
Layer.succeed(ProviderRegistry.ProviderRegistry, registry),
233-
Layer.succeed(ModelManifest.ModelManifest, {
234-
current: Effect.succeed(manifest),
235-
refresh: Effect.succeed(manifest),
236-
forceRefresh: Effect.succeed(manifest),
237-
refreshInBackground: Effect.void,
238-
}),
239-
// Fresh per runner so a version cached by one test cannot leak into another.
240-
Layer.sync(ProviderVersionCache, () => new Map()),
226+
Effect.service(ProviderMaintenanceRunner.ProviderMaintenanceRunner).pipe(
227+
Effect.provide(
228+
ProviderMaintenanceRunner.layer.pipe(
229+
Layer.provide(
230+
Layer.mergeAll(
231+
Layer.succeed(ProviderRegistry.ProviderRegistry, registry),
232+
Layer.succeed(ModelManifest.ModelManifest, {
233+
current: Effect.succeed(manifest),
234+
refresh: Effect.succeed(manifest),
235+
forceRefresh: Effect.succeed(manifest),
236+
refreshInBackground: Effect.void,
237+
}),
238+
// Fresh per runner so a version cached by one test cannot leak into another.
239+
Layer.sync(ProviderVersionCache, () => new Map()),
240+
),
241241
),
242242
),
243243
),
244-
).pipe(
245-
Effect.map((context) =>
246-
Context.get(context, ProviderMaintenanceRunner.ProviderMaintenanceRunner),
247-
),
248244
);
249245

250246
describe("providerMaintenanceRunner", () => {

‎packages/shared/src/KeyedLock.test.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,14 @@ describe("KeyedLock", () => {
9696
}),
9797
);
9898

99+
it.effect("keeps working after the scope that made it closes", () =>
100+
Effect.gen(function* () {
101+
// Services built per request hand their lock to work that outlives the build.
102+
const lock = yield* Effect.scoped(KeyedLock.make<string>());
103+
assert.strictEqual(yield* lock.withLock("key", Effect.succeed("ran")), "ran");
104+
}),
105+
);
106+
99107
it.effect("runs the effect in the caller's scope", () =>
100108
Effect.gen(function* () {
101109
const lock = yield* KeyedLock.make<string>();

‎packages/shared/src/KeyedLock.ts‎

Lines changed: 38 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
11
import * as Effect from "effect/Effect";
2-
import * as Exit from "effect/Exit";
3-
import * as RcMap from "effect/RcMap";
4-
import * as Scope from "effect/Scope";
52
import * as Semaphore from "effect/Semaphore";
63

74
export interface KeyedLock<Key> {
@@ -11,28 +8,47 @@ export interface KeyedLock<Key> {
118
readonly activeKeys: Effect.Effect<ReadonlyArray<Key>>;
129
}
1310

11+
interface LockEntry {
12+
readonly semaphore: Semaphore.Semaphore;
13+
users: number;
14+
}
15+
1416
/**
1517
* Mutual exclusion per key. A key's lock exists only while someone holds or
1618
* waits on it, so locks for keys that come and go never accumulate. Not
1719
* reentrant: taking a key while already holding it deadlocks.
1820
*
19-
* Keys compare by `Equal` (by value for strings and numbers). The lock lives as
20-
* long as the scope it is made in; after that scope closes, `withLock`
21-
* interrupts.
21+
* Keys compare like `Map` keys (by value for strings and numbers). The lock is
22+
* not tied to a scope, so it keeps working for as long as anyone references it.
2223
*/
23-
export const make = <Key>(): Effect.Effect<KeyedLock<Key>, never, Scope.Scope> =>
24-
Effect.map(RcMap.make({ lookup: (_key: Key) => Semaphore.make(1) }), (locks) => ({
25-
// The handle on the key's lock lives in its own scope, so `effect` still
26-
// runs in (and adds finalizers to) the caller's scope, not the lock's.
27-
withLock: (key, effect) =>
28-
Effect.acquireUseRelease(
29-
Scope.make(),
30-
(handle) =>
31-
RcMap.get(locks, key).pipe(
32-
Scope.provide(handle),
33-
Effect.flatMap((semaphore) => semaphore.withPermit(effect)),
34-
),
35-
(handle) => Scope.close(handle, Exit.void),
36-
),
37-
activeKeys: RcMap.keys(locks).pipe(Effect.map((keys) => Array.from(keys))),
38-
}));
24+
export const make = <Key>(): Effect.Effect<KeyedLock<Key>> =>
25+
Effect.sync(() => {
26+
const locks = new Map<Key, LockEntry>();
27+
28+
const acquire = (key: Key) =>
29+
Effect.sync(() => {
30+
let entry = locks.get(key);
31+
if (entry === undefined) {
32+
entry = { semaphore: Semaphore.makeUnsafe(1), users: 0 };
33+
locks.set(key, entry);
34+
}
35+
entry.users += 1;
36+
return entry;
37+
});
38+
39+
const release = (key: Key, entry: LockEntry) =>
40+
Effect.sync(() => {
41+
entry.users -= 1;
42+
if (entry.users === 0) locks.delete(key);
43+
});
44+
45+
return {
46+
withLock: (key, effect) =>
47+
Effect.acquireUseRelease(
48+
acquire(key),
49+
(entry) => entry.semaphore.withPermit(effect),
50+
(entry) => release(key, entry),
51+
),
52+
activeKeys: Effect.sync(() => Array.from(locks.keys())),
53+
};
54+
});

0 commit comments

Comments
 (0)