bug(timeline): a refused trace open leaves the writer lock held for good #2629

Written-by: Chief-audit

plugins/timeline/src/store.js takes the writer lock and then does work that can throw, but the only code that releases the lock is close(). After one refused read the directory stays locked for the process lifetime and the lock file stays on disk for every later process, so the trace never comes back on disk.

Where

• plugins/timeline/src/store.js:98 — lock = acquire(dir), and acquire() at :38-57 adds the directory to the in-process held Set (:45) and creates writer.lock (:43-44).
• plugins/timeline/src/store.js:99 — readdirSync(dir) right after the acquire, unguarded. A directory the process may not list (EACCES), a directory removed underneath (ENOENT), or an I/O error throws straight out of openTrace.
• plugins/timeline/src/store.js:160 — prune(), whose second half reads the directory again at :155, also unguarded.
• plugins/timeline/src/store.js:198-207 — close() is the only place that does held.delete(dir) (:204) and unlinkSync(lock) (:205). A throw between :98 and the returned object never reaches it.

Reproduction (on main 6f45177fe5299b225348a818e721efb5c50916e7, Node v22.23.3; the same code is read back from the repo, the script imports plugins/timeline/src/store.js directly)

$ chmod 0300 <dir>            # the directory exists but cannot be listed
$ node repro.mjs              # openTrace({ dir, persist: true }) three times
call 1: threw EACCES: EACCES: permission denied, scandir '<dir>'
lock file left on disk: true (writer.lock)
call 2 (same process, dir now listable again): refused TraceLocked: trace <dir> is already open in this process
call 3 (fresh node process): refused TraceLocked: another process (pid 830467) writes the trace in <dir>

The first failure is transient — it was a permission read — but nothing releases it: call 2 is refused by the in-process held Set, and every later process is refused by the writer.lock file the failed open left behind.

Why it matters in the product

plugins/timeline/src/timeline.js:19-26 wraps the open: on any throw it warns once and falls back to openTrace({ persist: false }) for the lifetime of the host (persisting = false, no retry). So one transient read or one leftover lock silently turns the on-disk trace off until restart — and after the restart the lock file is still there, so it stays off. The user sees a timeline that works and never persists, with one warning at start.

Expected

A failed open leaves nothing behind: the directory is not in held and the writer.lock file is removed, so a later openTrace on the same directory in the same process, and in any later process, succeeds once the underlying problem is gone. The throw itself must still reach the caller as it does today (timeline.js:21 decides the memory fallback; do not swallow it there).

Smallest fix

In openTrace, take the acquire into a try/catch that covers everything after it (:99-111 segment reading and the :160 prune()):

lock = acquire(dir)
try {
  const files = readdirSync(dir)...
  ...
  prune()
} catch (error) {
  held.delete(dir)
  try { unlinkSync(lock) } catch { /* already gone */ }
  throw error
}

Moving the early work into the same finally-style release that close() performs is fine too; what must not change is that a successful open still holds the lock until close().

Scope

plugins/timeline/src/store.js only — acquire()/close() semantics for a successful open, flush(), prune()'s retention rules and writer.lock takeover logic stay as they are. No retry loop, no new lock file format, no change to timeline.js's fallback.

Test

Extend the existing temp-dir test in plugins/timeline/test/ (no new harness): open a trace on a directory, make the directory unlistable (or remove it) so the open throws, assert the throw, then assert a second openTrace on the same directory in the same process succeeds once the directory is listable again and that a fresh process-level open would not see the leftover writer.lock (unlink it in the test between calls to a separate check). Red on main, green with the change.