fix(agents): memoize session lock owner args

openperf · web-flow · commit c430fcde1c42 · 2026-05-26T21:10:19.000+01:00
Memoize owner process argv lookups per PID during `cleanStaleLockFiles`, and yield between lock entries so startup cleanup does not monopolize the event loop while inspecting many session locks. This keeps lock classification semantics unchanged while avoiding repeated synchronous process-args reads for lock clusters owned by the same PID, especially the Windows PowerShell path. Fixes #86509. Verification: - `git diff --check origin/main...HEAD` - focused TSX harness against the current-main merge result: `session-lock memo regression harness passed` Thanks @openperf. Co-authored-by: openperf <16864032@qq.com>
diff --git a/src/agents/session-write-lock.test.ts b/src/agents/session-write-lock.test.ts
@@ -742,6 +742,78 @@ describe("acquireSessionWriteLock", () => {
     }
   });
 
+  it("memoizes readOwnerProcessArgs across locks with the same pid in one sweep (#86509)", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-"));
+    const sessionsDir = path.join(root, "sessions");
+    await fs.mkdir(sessionsDir, { recursive: true });
+    const nowMs = Date.now();
+    const lockCount = 5;
+    try {
+      for (let i = 0; i < lockCount; i++) {
+        await fs.writeFile(
+          path.join(sessionsDir, `same-pid-${i}.jsonl.lock`),
+          JSON.stringify({ pid: process.pid, createdAt: new Date(nowMs).toISOString() }),
+          "utf8",
+        );
+      }
+      const readArgsCalls: number[] = [];
+      const readOwnerProcessArgs = (pid: number) => {
+        readArgsCalls.push(pid);
+        return ["node", "/srv/app/dist/index.js"];
+      };
+      const result = await cleanStaleLockFiles({
+        sessionsDir,
+        staleMs: 30_000,
+        nowMs,
+        removeStale: true,
+        readOwnerProcessArgs,
+      });
+      expect(result.cleaned).toHaveLength(lockCount);
+      // Without memo this would be `lockCount`; the per-pid cache collapses it to a single call.
+      expect(readArgsCalls).toEqual([process.pid]);
+    } finally {
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("does not poison the per-pid memo when readOwnerProcessArgs throws (#86509)", async () => {
+    // A helper one layer up (`readOwnerProcessArgs`) already catches thrown resolvers and
+    // returns null, so `cleanStaleLockFiles` never propagates the throw — but a naive memo
+    // could still cache that null-equivalent failure and short-circuit later locks for the
+    // same pid. The fix writes the cache only after the resolver returns, so each lock
+    // retries the resolver fresh after a throw.
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-"));
+    const sessionsDir = path.join(root, "sessions");
+    await fs.mkdir(sessionsDir, { recursive: true });
+    const nowMs = Date.now();
+    const lockCount = 3;
+    try {
+      for (let i = 0; i < lockCount; i++) {
+        await fs.writeFile(
+          path.join(sessionsDir, `throwing-${i}.jsonl.lock`),
+          JSON.stringify({ pid: process.pid, createdAt: new Date(nowMs).toISOString() }),
+          "utf8",
+        );
+      }
+      let throwCalls = 0;
+      const result = await cleanStaleLockFiles({
+        sessionsDir,
+        staleMs: 30_000,
+        nowMs,
+        removeStale: true,
+        readOwnerProcessArgs: () => {
+          throwCalls++;
+          throw new Error("transient resolver failure");
+        },
+      });
+      // Resolver is invoked once per lock — the throw is not cached as a no-args entry.
+      expect(throwCalls).toBe(lockCount);
+      expect(result.cleaned).toHaveLength(0);
+    } finally {
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
   it("keeps fresh live .jsonl lock files with OpenClaw or unknown owners", async () => {
     const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-lock-"));
     const sessionsDir = path.join(root, "sessions");
diff --git a/src/agents/session-write-lock.ts b/src/agents/session-write-lock.ts
@@ -657,7 +657,20 @@ export async function cleanStaleLockFiles(params: {
   );
   const removeStale = params.removeStale !== false;
   const nowMs = params.nowMs ?? Date.now();
-  const ownerProcessArgsReader = params.readOwnerProcessArgs ?? readProcessArgsSync;
+  const baseOwnerProcessArgsReader = params.readOwnerProcessArgs ?? readProcessArgsSync;
+  // Memoize per-invocation: many locks in the same sweep often share a pid (gateway, MCP),
+  // and resolving owner argv is the most expensive per-lock syscall (PowerShell on Windows
+  // is ~0.5–1s per pid) — pids do not recycle within a single sweep. (#86509)
+  const ownerArgsByPid = new Map<number, string[] | null>();
+  const ownerProcessArgsReader: SessionLockOwnerProcessArgsReader = (pid) => {
+    const cached = ownerArgsByPid.get(pid);
+    if (cached !== undefined) {
+      return cached;
+    }
+    const args = baseOwnerProcessArgsReader(pid);
+    ownerArgsByPid.set(pid, args);
+    return args;
+  };
 
   let entries: fsSync.Dirent[] = [];
   try {
@@ -677,6 +690,11 @@ export async function cleanStaleLockFiles(params: {
     .toSorted((a, b) => a.name.localeCompare(b.name));
 
   for (const entry of lockEntries) {
+    // Yield to the event loop between locks so concurrent timers/HTTP polling can run
+    // while this sweep does per-lock sync syscalls (isPidAlive, /proc reads, PowerShell). (#86509)
+    await new Promise<void>((resolve) => {
+      setImmediate(resolve);
+    });
     const lockPath = path.join(sessionsDir, entry.name);
     const payload = await readLockPayload(lockPath);
     const inspected = inspectLockPayloadForSession({
