Folder: a mutation waits for this process's own short read instead of failing busy (#272)

src/folder/lock.ts

@@ -1,6 +1,6 @@
 import { constants } from "node:fs";
 import { lstat, mkdir, open, readdir, statfs } from "node:fs/promises";
-import { join } from "node:path";
+import { join, resolve } from "node:path";
 import {
   closeOnExecFlag,
   darwinFilesystemName,
@@ -155,15 +155,67 @@ export async function acquireFolderOperationLock(stateDirectory: string) {
   }
 }
 
+/** This process's short reads that hold the lock now, by state directory.
+ * A mutation of this process waits a bounded moment for them (issue #272):
+ * the shell setup cache starts its background read right after a profile
+ * change, and the person's next change must not fail as busy because of the
+ * Launchpad's own read. It never waits for another process. */
+const ownReads = new Map<string, number>();
+
+/** How long a mutation of this process waits for its own short reads. */
+export const ownReadWaitMs = 3_000;
+
+/** A mutation under the lock. It refuses at once when another process holds
+ * the lock, or this process's own mutation does; only this process's short
+ * read (`withFolderReadLock`) is waited for, `waitForOwnReadsMs` at most. */
 export async function withFolderOperationLock<T>(
   stateDirectory: string,
   operation: (assertHeld: () => Promise<void>) => Promise<T>,
+  waitForOwnReadsMs = ownReadWaitMs,
+): Promise<T> {
+  const deadline = Date.now() + waitForOwnReadsMs;
+  for (let attempt = 1; ; attempt += 1) {
+    let lock: Awaited<ReturnType<typeof acquireFolderOperationLock>>;
+    try {
+      lock = await acquireFolderOperationLock(stateDirectory);
+    } catch (error) {
+      if (
+        !(error instanceof FolderOperationBusyError) ||
+        (ownReads.get(resolve(stateDirectory)) ?? 0) === 0 ||
+        Date.now() >= deadline
+      )
+        throw error;
+      await Bun.sleep(Math.min(10 * attempt, 100));
+      continue;
+    }
+    try {
+      return await operation(lock.assertHeld);
+    } finally {
+      await lock.release();
+    }
+  }
+}
+
+/** Holds the lock for one short read, counted as this process's own until
+ * the lock is released: a mutation that finds the lock busy meanwhile knows
+ * whom it waits for. */
+async function ownRead<T>(
+  stateDirectory: string,
+  operation: (assertHeld: () => Promise<void>) => Promise<T>,
 ): Promise<T> {
   const lock = await acquireFolderOperationLock(stateDirectory);
+  const key = resolve(stateDirectory);
+  ownReads.set(key, (ownReads.get(key) ?? 0) + 1);
   try {
     return await operation(lock.assertHeld);
   } finally {
-    await lock.release();
+    try {
+      await lock.release();
+    } finally {
+      const left = (ownReads.get(key) ?? 1) - 1;
+      if (left > 0) ownReads.set(key, left);
+      else ownReads.delete(key);
+    }
   }
 }
 
@@ -172,8 +224,9 @@ export async function withFolderOperationLock<T>(
  * rule's preset reads of a running gh session, a poll): the lock is
  * exclusive and non-blocking, so two such reads at the same moment would
  * refuse one of them as busy. A read waits for the holder instead, a
- * bounded time (default 3 s), and then fails as before. Mutations keep the
- * immediate refusal of `withFolderOperationLock`. */
+ * bounded time (default 3 s), and then fails as before. A mutation of this
+ * process waits for such a read (`withFolderOperationLock`); one of another
+ * process is refused at once. */
 export async function withFolderReadLock<T>(
   stateDirectory: string,
   operation: (assertHeld: () => Promise<void>) => Promise<T>,
@@ -182,7 +235,7 @@ export async function withFolderReadLock<T>(
   const deadline = Date.now() + waitMs;
   for (let attempt = 1; ; attempt += 1) {
     try {
-      return await withFolderOperationLock(stateDirectory, operation);
+      return await ownRead(stateDirectory, operation);
     } catch (error) {
       if (
         !(error instanceof FolderOperationBusyError) ||

tests/folder-lock.test.ts

@@ -243,3 +243,77 @@ test.skipIf(process.platform === "win32")(
     }
   },
 );
+
+// Issue #272: the shell setup cache reads the Folder in the background right
+// after a profile change; the person's next change waits for that read.
+test.skipIf(process.platform === "win32")(
+  "a mutation of this process waits for its own short read, never for another holder",
+  async () => {
+    const directory = await realpath(
+      await mkdtemp(join(tmpdir(), "lock-own-read-")),
+    );
+    try {
+      let releaseRead = () => {};
+      const reading = new Promise<void>((resolve) => {
+        releaseRead = resolve;
+      });
+      const order: string[] = [];
+      const read = withFolderReadLock(directory, async () => {
+        order.push("read");
+        await reading;
+        order.push("read done");
+      });
+      await Bun.sleep(30);
+      const write = withFolderOperationLock(directory, async () => {
+        order.push("write");
+      });
+      await Bun.sleep(100);
+      expect(order).toEqual(["read"]);
+      releaseRead();
+      await Promise.all([read, write]);
+      expect(order).toEqual(["read", "read done", "write"]);
+
+      // Any other holder (here one taken without the read) refuses at once.
+      const held = await acquireFolderOperationLock(directory);
+      try {
+        const started = Date.now();
+        await expect(
+          withFolderOperationLock(directory, async () => {}),
+        ).rejects.toThrow("busy");
+        expect(Date.now() - started).toBeLessThan(500);
+      } finally {
+        await held.release();
+      }
+    } finally {
+      await rm(directory, { recursive: true, force: true });
+    }
+  },
+);
+
+test.skipIf(process.platform === "win32")(
+  "the wait for an own read is bounded, and the mutation then fails as busy",
+  async () => {
+    const directory = await realpath(
+      await mkdtemp(join(tmpdir(), "lock-own-read-bound-")),
+    );
+    try {
+      let releaseRead = () => {};
+      const reading = new Promise<void>((resolve) => {
+        releaseRead = resolve;
+      });
+      const read = withFolderReadLock(directory, () => reading);
+      await Bun.sleep(30);
+      const started = Date.now();
+      await expect(
+        withFolderOperationLock(directory, async () => {}, 150),
+      ).rejects.toThrow("busy");
+      expect(Date.now() - started).toBeGreaterThanOrEqual(140);
+      releaseRead();
+      await read;
+      // Free again: a mutation goes through at once.
+      await withFolderOperationLock(directory, async () => {});
+    } finally {
+      await rm(directory, { recursive: true, force: true });
+    }
+  },
+);