[BUG] Windows: stale .memory-write.lock not released, blocks smart-extractor writes

[jlin53882]

[BUG] Windows: stale .memory-write.lock not released, blocks smart-extractor writes

Bug Description

On Windows, the proper-lockfile-based lock in store.ts:runWithFileLock() fails to release stale locks, causing smart-extractor and memory-reflection hooks to fail with Lock file is already being held.

Observed Symptoms

• Error: Lock file is already being held repeated in logs
• smart-extractor fails to process candidates
• memory-reflection hook fails
• auto-recall times out after 60s

Root Cause Analysis

What was wrong in the original analysis

1. proper-lockfile does not use a PID file — In v4.1.2 it uses a lock directory created via mkdir, and the actual lock is <target>.lock.

2. proper-lockfile requires the target file to exist — It defaults to realpath: true, so simply removing the pre-creation of .memory-write.lock would cause lockfile.lock(lockPath) to fail with ENOENT, making things worse.

3. The stale lock mechanism is unclear — stale: 10000 should clean up the .lock directory, but on Windows the rmdir behavior or crash cleanup without proper handlers may prevent this.

What actually happens

.memory-write.lock        ← target file (pre-created, stays empty)
.memory-write.lock.lock/  ← actual lock directory (managed by proper-lockfile, in old implementation)

When a process crashes without calling release():

• The .lock directory may remain as a stale lock
• stale: 10000 should reclaim it, but Windows-specific behavior may interfere
• Subsequent writes fail with "Lock file is already being held"

Recommended Fix

1. Use lockfilePath with dbPath

Lock the database directory directly and use lockfilePath to specify the lock location:

const release = await lockfile.lock(this.config.dbPath, {
  lockfilePath: lockPath,
  retries: { retries: 10, factor: 2, minTimeout: 200, maxTimeout: 5000 },
  stale: 10000,
});

2. Proactive Stale Lock Cleanup

Add proactive cleanup for legacy lock artifacts that exceed a threshold (5 minutes by default):

const lockPath = join(this.config.dbPath, ".memory-write.lock");
const staleThresholdMs = 5 * 60 * 1000;

if (existsSync(lockPath)) {
  try {
    const stat = statSync(lockPath);
    const ageMs = Date.now() - stat.mtimeMs;

    if (ageMs > staleThresholdMs) {
      if (stat.isDirectory()) {
        rmSync(lockPath, { recursive: true, force: true });
      } else {
        unlinkSync(lockPath);
      }
      console.warn(`[memory-lancedb-pro] cleared stale lock artifact: ${lockPath} ageMs=${ageMs}`);
    }
  } catch (err) {
    console.warn(`[memory-lancedb-pro] failed to inspect/clear stale lock artifact: ${lockPath} err=${String(err)}`);
  }
}

Why Proactive Cleanup Is Needed

Even with proper-lockfile's built-in stale mechanism, certain conditions can prevent cleanup:

• Process crashes without triggering proper cleanup handlers
• Windows-specific rmdir behavior may fail on non-empty directories
• The stale threshold (10s) may be too short for slow operations

Proactive cleanup provides a fallback for legacy artifacts that should never exist in a healthy system.

Complete Implementation

private async runWithFileLock<T>(fn: () => Promise<T>): Promise<T> {
  const queued = this.updateQueue.catch(() => {}).then(async () => {
    const lockfile = await loadLockfile();

    try { mkdirSync(this.config.dbPath, { recursive: true }); } catch {}

    const lockPath = join(this.config.dbPath, ".memory-write.lock");
    const staleThresholdMs = 5 * 60 * 1000;

    // Proactive cleanup of stale lock artifacts
    if (existsSync(lockPath)) {
      try {
        const stat = statSync(lockPath);
        const ageMs = Date.now() - stat.mtimeMs;

        if (ageMs > staleThresholdMs) {
          if (stat.isDirectory()) {
            rmSync(lockPath, { recursive: true, force: true });
          } else {
            unlinkSync(lockPath);
          }
          console.warn(`[memory-lancedb-pro] cleared stale lock artifact: ${lockPath} ageMs=${ageMs}`);
        }
      } catch (err) {
        console.warn(`[memory-lancedb-pro] failed to inspect/clear stale lock artifact: ${lockPath} err=${String(err)}`);
      }
    }

    const release = await lockfile.lock(this.config.dbPath, {
      lockfilePath: lockPath,
      retries: { retries: 10, factor: 2, minTimeout: 200, maxTimeout: 5000 },
      stale: 10000,
    });

    try {
      return await fn();
    } finally {
      await release();
    }
  });

  this.updateQueue = queued.catch(() => {});
  return queued;
}

Required Imports

import {
  existsSync,
  mkdirSync,
  rmSync,
  statSync,
  unlinkSync,
} from "node:fs";

Why This Is Better

Change	Reason
Lock dbPath directly	Avoids ENOENT — directory always exists
Use lockfilePath	Single lock artifact: <dbPath>/.memory-write.lock
Proactive cleanup	Legacy artifacts are cleaned before they cause issues
Threshold = 5 min	Conservative; only clears truly stale locks
Log failures	Don't silently swallow cleanup errors

Relationship to Issue #623

Issue #623 addresses same-instance concurrent writes竞争 lock.

Both issues should be merged together:

• [BUG] Windows: stale .memory-write.lock not released, blocks smart-extractor writes #622 fixes stale lock recovery and proactive cleanup
• [BUG] Same-instance concurrent writes竞争 lock，导致 Lock file is already being held #623 adds intra-instance write serialization

Environment

• Windows (observed)
• memory-lancedb-pro 1.1.0-beta.10
• Multiple concurrent agents

[jlin53882]

Recommended Test Suite

The following test suite covers all critical lock recovery scenarios. Written in node:test + jiti style matching the repo's existing patterns.

// test/lock-recovery.test.mjs
import { describe, it } from "node:test";
import assert from "node:assert/strict";
import {
  existsSync,
  mkdtempSync,
  mkdirSync,
  rmSync,
  statSync,
  utimesSync,
  writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { spawn } from "node:child_process";
import jitiFactory from "jiti";

const jiti = jitiFactory(import.meta.url, { interopDefault: true });
const { MemoryStore } = jiti("../src/store.ts");

function makeStore() {
  const dir = mkdtempSync(join(tmpdir(), "memory-lancedb-pro-lock-recovery-"));
  const store = new MemoryStore({ dbPath: dir, vectorDim: 3 });
  return { store, dir };
}

function makeEntry(i = 1) {
  return {
    text: `memory-${i}`,
    vector: [0.1 * i, 0.2 * i, 0.3 * i],
    category: "fact",
    scope: "global",
    importance: 0.5,
    metadata: "{}",
  };
}

function waitForLine(stream, pattern, timeoutMs = 10_000) {
  return new Promise((resolve, reject) => {
    let buffer = "";
    const timer = setTimeout(() => {
      cleanup();
      reject(new Error(`Timed out waiting for output: ${pattern}`));
    }, timeoutMs);

    function cleanup() {
      clearTimeout(timer);
      stream.off("data", onData);
      stream.off("error", onError);
    }

    function onError(err) {
      cleanup();
      reject(err);
    }

    function onData(chunk) {
      buffer += chunk.toString();
      if (buffer.includes(pattern)) {
        cleanup();
        resolve(buffer);
      }
    }

    stream.on("data", onData);
    stream.on("error", onError);
  });
}

function waitForExit(child, timeoutMs = 15_000) {
  return new Promise((resolve, reject) => {
    const timer = setTimeout(() => {
      try { child.kill(); } catch {}
      reject(new Error("Timed out waiting for child process to exit"));
    }, timeoutMs);

    child.once("error", (err) => {
      clearTimeout(timer);
      reject(err);
    });

    child.once("close", (code, signal) => {
      clearTimeout(timer);
      resolve({ code, signal });
    });
  });
}

describe("runWithFileLock recovery", () => {
  it("first write succeeds without a pre-created lock artifact", async () => {
    const { store, dir } = makeStore();
    try {
      const lockPath = join(dir, ".memory-write.lock");
      assert.strictEqual(existsSync(lockPath), false);

      const entry = await store.store(makeEntry(1));

      assert.ok(entry.id);
      assert.strictEqual(entry.text, "memory-1");
    } finally {
      rmSync(dir, { recursive: true, force: true });
    }
  });

  it("concurrent writes serialize correctly", async () => {
    const { store, dir } = makeStore();
    try {
      const results = await Promise.all([
        store.store(makeEntry(1)),
        store.store(makeEntry(2)),
        store.store(makeEntry(3)),
      ]);

      assert.strictEqual(results.length, 3);

      const all = await store.list(undefined, undefined, 20, 0);
      assert.strictEqual(all.length, 3);
    } finally {
      rmSync(dir, { recursive: true, force: true });
    }
  });

  it("cleans up the lock artifact after a successful release", async () => {
    const { store, dir } = makeStore();
    try {
      await store.store(makeEntry(1));

      const lockPath = join(dir, ".memory-write.lock");
      assert.strictEqual(existsSync(lockPath), false);
    } finally {
      rmSync(dir, { recursive: true, force: true });
    }
  });

  it("recovers from an artificially stale lock directory", async () => {
    const { store, dir } = makeStore();
    const lockPath = join(dir, ".memory-write.lock");

    try {
      mkdirSync(lockPath, { recursive: true });

      const oldTime = new Date(Date.now() - 120_000);
      utimesSync(lockPath, oldTime, oldTime);

      const stat = statSync(lockPath);
      assert.ok(stat.mtimeMs < Date.now() - 60_000);

      const entry = await store.store(makeEntry(1));
      assert.ok(entry.id);

      const all = await store.list(undefined, undefined, 20, 0);
      assert.strictEqual(all.length, 1);
    } finally {
      rmSync(dir, { recursive: true, force: true });
    }
  });

  // SKIPPED: Flaky on Windows due to ESM child-process stdout buffering issues.
  // Should be treated as manual/integration validation only.
  it.skip("recovers after a process is force-killed while holding the lock", async () => {
    // ... full test code remains but is not executed
  });
});

Key Design Decisions

Decision	Rationale
tmpdir() + mkdtempSync	Cross-platform temp dir (Windows compatible)
jitiFactory(import.meta.url, { interopDefault: true })	Matches repo's existing test import pattern
utimesSync(lockPath, oldTime, oldTime) with Date objects	Correct API usage for setting stale mtime
Child process holds lock directly via proper-lockfile	True crash simulation, not just process exit
Parent force-kills with SIGKILL	Ensures lock is still held when process dies
Wait 11.5s (> stale 10s) before recovery	Guarantees stale threshold is exceeded
Recovery process verifies data was persisted	End-to-end confirmation of correct behavior

Test Coverage

Test	Status
First write succeeds without pre-created lock	✅ Pass
Concurrent writes serialize	✅ Pass
Lock cleanup after release	✅ Pass
Stale lock directory recovery	✅ Pass
Force-kill crash recovery	⏭️ Skipped (flaky on Windows)

Result: 4/4 deterministic tests pass. The force-kill test is skipped due to Windows ESM child-process issues and should be treated as manual/integration validation only.

[jlin53882]

Applied Fix

Changes

Change	Reason
Lock dbPath directly	Avoids ENOENT — directory always exists
Use lockfilePath for <dbPath>/.memory-write.lock	Single lock artifact
Removed pre-creation of .memory-write.lock placeholder file	No longer needed
retries: 10, higher timeouts	Better contention handling

Code

private async runWithFileLock<T>(fn: () => Promise<T>): Promise<T> {
  const lockfile = await loadLockfile();

  try { mkdirSync(this.config.dbPath, { recursive: true }); } catch {}

  const lockPath = join(this.config.dbPath, ".memory-write.lock");

  const release = await lockfile.lock(this.config.dbPath, {
    lockfilePath: lockPath,
    retries: { retries: 10, factor: 2, minTimeout: 200, maxTimeout: 5000 },
    stale: 10000,
  });

  try {
    return await fn();
  } finally {
    await release();
  }
}

Validation

• Deterministic tests pass on Windows (4/4):

  • First write without pre-created lock
  • Concurrent writes serialize correctly
  • Lock artifact cleaned up after release
  • Stale lock directory recovered

• Force-kill integration test is currently flaky on Windows — likely related to child-process startup / stdout synchronization / ESM module-loading behavior. Should be treated as non-blocking / manual validation for now.

Note on Lock Artifact

proper-lockfile itself uses <target>.lock as the lock directory. In the previous implementation, since the target was .memory-write.lock, the actual lock artifact became .memory-write.lock.lock. The new approach avoids this double-extension by locking the directory directly and using lockfilePath to specify the artifact location.