History

Shared executor should wait behind a lifecycle hook so getExecutor() cannot reopen while a previous handle is still disposing

Griffin Evans committed 4 months ago

Commit d8fe55b1 parent 636e7092 files · +71 −7Browse files

Unified · Split

2 files
  1. apps/local/src/executor.test.ts +54 −0
  2. apps/local/src/executor.ts +17 −7
apps/local/src/executor.test.ts+54−0
@@ -0,0 +1,54 @@
1+/* oxlint-disable executor/no-try-catch-or-throw -- test boundary: isolate process env and always dispose the shared executor handle */
2+import { mkdtempSync, rmSync } from "node:fs";
3+import { tmpdir } from "node:os";
4+import { join } from "node:path";
5+
6+import { describe, expect, it } from "@effect/vitest";
7+
8+import { disposeExecutor, getExecutor, reloadExecutor } from "./executor";
9+
10+const withIsolatedExecutorDataDir = async (body: () => Promise<void>): Promise<void> => {
11+ const previousDataDir = process.env.EXECUTOR_DATA_DIR;
12+ const previousScopeDir = process.env.EXECUTOR_SCOPE_DIR;
13+ const dataDir = mkdtempSync(join(tmpdir(), "executor-reload-race-"));
14+
15+ process.env.EXECUTOR_DATA_DIR = dataDir;
16+ process.env.EXECUTOR_SCOPE_DIR = dataDir;
17+
18+ try {
19+ await body();
20+ } finally {
21+ await disposeExecutor();
22+ if (previousDataDir === undefined) {
23+ delete process.env.EXECUTOR_DATA_DIR;
24+ } else {
25+ process.env.EXECUTOR_DATA_DIR = previousDataDir;
26+ }
27+ if (previousScopeDir === undefined) {
28+ delete process.env.EXECUTOR_SCOPE_DIR;
29+ } else {
30+ process.env.EXECUTOR_SCOPE_DIR = previousScopeDir;
31+ }
32+ rmSync(dataDir, { recursive: true, force: true });
33+ }
34+};
35+
36+describe("reloadExecutor", () => {
37+ it("waits for the previous owned database handle to release before reopening", async () => {
38+ await withIsolatedExecutorDataDir(async () => {
39+ await getExecutor();
40+ const executor = await reloadExecutor();
41+ expect(executor).toBeDefined();
42+ });
43+ });
44+
45+ it("serializes new shared executor opens behind an in-flight dispose", async () => {
46+ await withIsolatedExecutorDataDir(async () => {
47+ await getExecutor();
48+ const disposing = disposeExecutor();
49+ const executor = await getExecutor();
50+ await disposing;
51+ expect(executor).toBeDefined();
52+ });
53+ });
54+});
apps/local/src/executor.ts+17−7
@@ -256,10 +256,11 @@ export const createExecutorHandle = async (options: LocalExecutorOptions = {}) = · 255 unmodified lines
256256 export type ExecutorHandle = Awaited<ReturnType<typeof createExecutorHandle>>;
257257
258258 let sharedHandlePromise: ReturnType<typeof createExecutorHandle> | null = null;
259+let sharedHandleLifecycle: Promise<void> = Promise.resolve();
259260
260261 const loadSharedHandle = () => {
261262 if (!sharedHandlePromise) {
262− sharedHandlePromise = createExecutorHandle();
263+ sharedHandlePromise = sharedHandleLifecycle.then(() => createExecutorHandle());
263264 }
264265 return sharedHandlePromise;
265266 };
@@ -271,13 +272,22 @@ export const disposeExecutor = async (): Promise<void> => { · 5 unmodified lines
271272 const currentHandlePromise = sharedHandlePromise;
272273 sharedHandlePromise = null;
273274
274− const handle = currentHandlePromise ? await handleOrNull(currentHandlePromise) : null;
275− if (handle) {
276− await ignorePromiseFailure("disposeExecutor", () => handle.dispose());
277− }
275+ const disposeCurrent = async (): Promise<void> => {
276+ const handle = currentHandlePromise ? await handleOrNull(currentHandlePromise) : null;
277+ if (handle) {
278+ await ignorePromiseFailure("disposeExecutor", () => handle.dispose());
279+ }
280+ };
281+
282+ const nextLifecycle = sharedHandleLifecycle.then(disposeCurrent, disposeCurrent);
283+ sharedHandleLifecycle = nextLifecycle.then(
284+ () => undefined,
285+ () => undefined,
286+ );
287+ await nextLifecycle;
278288 };
279289
280−export const reloadExecutor = () => {
281− disposeExecutor();
290+export const reloadExecutor = async () => {
291+ await disposeExecutor();
282292 return getExecutor();
283293 };