From 3768c03653caa428f247f8b9521c4c359dd86e14 Mon Sep 17 00:00:00 2001 From: Yann Roberto <1922498827@qq.com> Date: Thu, 1 Oct 2026 01:03:41 +0800 Subject: [PATCH] fix(state-store): preserve permission errors after lock retries --- .../hud-status-session-control.md | 8 +++-- scripts/lib/state-store/file-lock.js | 7 ++-- .../helpers/control-pane-contention-server.js | 23 ++++++++---- tests/lib/state-store-persistence.test.js | 36 ++++++++++++------- 4 files changed, 51 insertions(+), 23 deletions(-) diff --git a/docs/architecture/hud-status-session-control.md b/docs/architecture/hud-status-session-control.md index 37f745187..0787c0003 100644 --- a/docs/architecture/hud-status-session-control.md +++ b/docs/architecture/hud-status-session-control.md @@ -97,8 +97,12 @@ remove the database itself. In-memory stores do not create lock files. The control-pane HTTP server runs board claims and moves in one-shot workers, so waiting for another writer does not stall health checks or snapshots. A -timed-out mutation returns HTTP 503 with `code: STATE_STORE_BUSY` and -`Retry-After: 1`; invalid mutations continue to return HTTP 400. Each worker +mutation that times out while lock creation reports `EEXIST` returns HTTP 503 +with `code: STATE_STORE_BUSY` and `Retry-After: 1`. On Windows, lock creation +also retries `EPERM` within the same deadline because deletion may still be +pending. If the final attempt still reports `EPERM`, the original permission +error is preserved rather than replaced with busy or leftover-lock recovery +advice. Invalid mutations continue to return HTTP 400. Each worker closes its store before reporting the result and exits naturally, including when the requesting browser disconnects, to avoid interrupting a write. diff --git a/scripts/lib/state-store/file-lock.js b/scripts/lib/state-store/file-lock.js index 02a18c533..5e35fa5f9 100644 --- a/scripts/lib/state-store/file-lock.js +++ b/scripts/lib/state-store/file-lock.js @@ -75,10 +75,13 @@ function acquireLock(dbPath, timeoutMs) { } catch (error) { // Windows may report EPERM while the previous owner's unlinked file is // pending deletion. Retry exclusive creation within the same deadline. - const pendingDelete = process.platform === 'win32' && hasCode(error, 'EPERM'); - if (!hasCode(error, 'EEXIST') && !pendingDelete) throw error; + const retryablePermissionError = process.platform === 'win32' && hasCode(error, 'EPERM'); + if (!hasCode(error, 'EEXIST') && !retryablePermissionError) throw error; const remaining = deadline - performance.now(); if (remaining <= 0) { + // EPERM can also mean genuine access denial; preserve its diagnostics + // rather than advising the caller to remove a possibly unrelated lock. + if (retryablePermissionError) throw error; const busy = new Error(`State store is busy: ${dbPath}. Retry after the other ECC operation finishes. ` + `If an operation terminated unexpectedly, stop all ECC processes using this database, ` + `then inspect and remove the leftover lock: ${lockPath}`); diff --git a/tests/lib/helpers/control-pane-contention-server.js b/tests/lib/helpers/control-pane-contention-server.js index 516db59ad..9f70ba0d2 100644 --- a/tests/lib/helpers/control-pane-contention-server.js +++ b/tests/lib/helpers/control-pane-contention-server.js @@ -2,7 +2,7 @@ const workerThreads = require('worker_threads'); const OriginalWorker = workerThreads.Worker; -const metrics = { active: 0, peak: 0, started: 0, exited: 0, received: 0, disconnected: 0 }; +let metrics = { active: 0, peak: 0, started: 0, exited: 0, received: 0, disconnected: 0 }; function send(message) { if (process.connected) process.send(message, error => { if (error) process.exitCode = 1; }); } @@ -10,11 +10,17 @@ function report() { send({ type: 'workers', ...metrics }); } workerThreads.Worker = class ObservedWorker extends OriginalWorker { constructor(...args) { super(...args); - metrics.active += 1; - metrics.started += 1; - metrics.peak = Math.max(metrics.peak, metrics.active); + metrics = { + ...metrics, + active: metrics.active + 1, + started: metrics.started + 1, + peak: Math.max(metrics.peak, metrics.active + 1) + }; report(); - this.once('exit', () => { metrics.active -= 1; metrics.exited += 1; report(); }); + this.once('exit', () => { + metrics = { ...metrics, active: metrics.active - 1, exited: metrics.exited + 1 }; + report(); + }); } }; const { createControlPaneServer } = require('../../../scripts/lib/control-pane/server'); @@ -26,11 +32,14 @@ async function main() { }); app.server.prependListener('request', (req, res) => { if (req.method === 'POST') { - metrics.received += 1; + metrics = { ...metrics, received: metrics.received + 1 }; report(); send({ type: 'mutation-received' }); res.once('close', () => { - if (!res.writableFinished) { metrics.disconnected += 1; report(); } + if (!res.writableFinished) { + metrics = { ...metrics, disconnected: metrics.disconnected + 1 }; + report(); + } }); } }); diff --git a/tests/lib/state-store-persistence.test.js b/tests/lib/state-store-persistence.test.js index a6d5609da..c2bc046a0 100644 --- a/tests/lib/state-store-persistence.test.js +++ b/tests/lib/state-store-persistence.test.js @@ -27,7 +27,7 @@ function thrownBy(fn) { catch (error) { return { failed: true, error }; } } -function lockFixture(overrides = {}, platform = 'linux') { +function lockFixture(overrides = {}, platform = 'linux', performance = require('perf_hooks').performance) { const calls = []; const identity = { dev: 1n, ino: 2n, isFile: () => true, isSymbolicLink: () => false }; let present = true; @@ -46,7 +46,7 @@ function lockFixture(overrides = {}, platform = 'linux') { const original = facade[name]; facade[name] = (...args) => fn(state, original, ...args); } - const lock = loadPrivate(LOCK, { fs: facade, os: { hostname: () => 'fixture' } }, undefined, + const lock = loadPrivate(LOCK, { fs: facade, os: { hostname: () => 'fixture' }, perf_hooks: { performance } }, undefined, { platform, pid: 7 }); return { ...state, calls, run: (callback, timeoutMs = 0) => lock.withStateStoreLock('synthetic.db', callback, { timeoutMs }) }; } @@ -69,16 +69,28 @@ async function runSynthetic(test) { assert.strictEqual(callbacks, 1); assert.deepStrictEqual(fixture.calls, ['open', 'open', 'fstat', 'write', 'lstat', 'unlink', 'close']); }); - await test('sustained Windows open EPERM times out as busy without touching the lock', () => { - const denied = Object.assign(new Error('pending deletion'), { code: 'EPERM' }); - const fixture = lockFixture({ openSync(state) { state.calls.push('open'); throw denied; } }, 'win32'); - const result = thrownBy(() => fixture.run(() => assert.fail('callback must not run'), 20)); - assert.ok(result.failed); - assert.strictEqual(result.error.code, 'STATE_STORE_BUSY'); - assert.match(result.error.message, /synthetic\.db\.ecc-state\.lock/); - assert.ok(fixture.calls.length > 0); - assert.ok(fixture.calls.every(call => call === 'open')); - }); + for (const codes of [['EPERM', 'EPERM'], ['EEXIST', 'EPERM'], ['EPERM', 'EEXIST']]) { + await test(`Windows lock timeout preserves the final error classification (${codes.join(' -> ')})`, () => { + const errors = codes.map(code => Object.assign(new Error(`open failed: ${code}`), { code })); + let attempts = 0; + const clock = { now: () => attempts * 10 }; + const fixture = lockFixture({ openSync(state) { + state.calls.push('open'); + throw errors[attempts++]; + } }, 'win32', clock); + const result = thrownBy(() => fixture.run(() => assert.fail('callback must not run'), 20)); + assert.ok(result.failed); + if (codes[1] === 'EPERM') { + assert.strictEqual(result.error, errors[1]); + assert.doesNotMatch(result.error.message, /remove|leftover lock/i); + } else { + assert.strictEqual(result.error.code, 'STATE_STORE_BUSY'); + assert.match(result.error.message, /synthetic\.db\.ecc-state\.lock/); + } + assert.strictEqual(attempts, 2); + assert.deepStrictEqual(fixture.calls, ['open', 'open']); + }); + } for (const [platform, code] of [['linux', 'EPERM'], ['darwin', 'EPERM'], ['win32', 'EACCES'], ['win32', 'EIO']]) { await test(`unrelated lock-open errors propagate immediately (${platform}, ${code})`, () => { const denied = Object.assign(new Error('open denied'), { code });