fix: race-proof writer-lock acquisition + machine-readable conflict through init
- acquireWriterLock now CLAIMS with an atomic create-exclusive write (O_EXCL) inside a bounded retry loop: two processes racing an absent lock can never both succeed (the old read-then-tmp-rename flow let the loser keep running unlocked, silently). An EEXIST loser re-evaluates and either throws loudly with the winner's details or performs a verified stale-takeover (re-read before unlink so a lock that changed hands mid-deliberation is never clobbered). Exhausted contention fails loudly instead of degrading into a lockless open. - BRAINY_WRITER_LOCKED passes through init() unwrapped: the error documents a machine-readable contract (err.code + err.lockInfo with the holder's pid/host/heartbeat), but init's blanket error wrapping stripped both, leaving consumers a message to regex against. - Two contract tests added: stale-foreign takeover installs OUR lock via the atomic claim; the conflict error carries code + lockInfo at the public init() surface.
This commit is contained in:
parent
e450e0eedf
commit
01a3b46ade
4 changed files with 188 additions and 49 deletions
|
|
@ -1373,6 +1373,13 @@ export class Brainy<T = any> implements BrainyInterface<T> {
|
|||
if (this._readyReject) {
|
||||
this._readyReject(error instanceof Error ? error : new Error(String(error)))
|
||||
}
|
||||
// Machine-readable init failures pass through UNWRAPPED — the writer-lock
|
||||
// conflict documents an err.code/err.lockInfo contract ("callers detect
|
||||
// this case via err.code"), and wrapping in a fresh Error silently
|
||||
// stripped both, leaving consumers only a message to regex against.
|
||||
if (error instanceof Error && (error as Error & { code?: string }).code === 'BRAINY_WRITER_LOCKED') {
|
||||
throw error
|
||||
}
|
||||
throw new Error(`Failed to initialize Brainy: ${error}`)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1764,64 +1764,117 @@ export class FileSystemStorage extends BaseStorage {
|
|||
const lockFile = path.join(this.lockDir, FileSystemStorage.WRITER_LOCK_FILE)
|
||||
const os = await import('node:os')
|
||||
const hostname = os.hostname()
|
||||
const now = new Date().toISOString()
|
||||
const myPid = typeof process !== 'undefined' && process.pid ? process.pid : 0
|
||||
|
||||
const existing = await this.readWriterLock()
|
||||
if (existing && !options?.force) {
|
||||
// Same-process re-open: a second Brainy instance in this Node process
|
||||
// (e.g. test "simulate server restart" patterns, or a consumer that
|
||||
// explicitly re-instantiates without closing first). This isn't the
|
||||
// dangerous cross-process case the lock exists to prevent — the two
|
||||
// instances share a memory space and can't silently diverge from each
|
||||
// other beyond what their callers already see. Warn and take over the lock.
|
||||
if (existing.pid === (typeof process !== 'undefined' ? process.pid : 0) &&
|
||||
existing.hostname === hostname) {
|
||||
console.warn(
|
||||
`[brainy] Re-acquiring writer lock for ${this.rootDir} held by the same process (PID ${existing.pid}). ` +
|
||||
`If you intended to keep the previous Brainy instance alive, this is a bug — close it first.`
|
||||
)
|
||||
} else {
|
||||
const stale = await this.isWriterLockStale(existing)
|
||||
if (!stale) {
|
||||
// Bounded acquire loop. The CLAIM itself is an atomic create-exclusive
|
||||
// write (O_EXCL) — two processes racing an ABSENT lock can never both
|
||||
// succeed, which closes the read-then-write window where the loser used
|
||||
// to keep running unlocked, silently. An EEXIST loser loops, re-reads,
|
||||
// and handles whatever it finds honestly (fresh foreign lock → loud
|
||||
// throw; stale/forced → verified takeover).
|
||||
const MAX_ATTEMPTS = 3
|
||||
for (let attempt = 0; attempt < MAX_ATTEMPTS; attempt++) {
|
||||
const now = new Date().toISOString()
|
||||
const existing = await this.readWriterLock()
|
||||
|
||||
if (existing) {
|
||||
// Same-process re-open: a second Brainy instance in this Node process
|
||||
// (e.g. test "simulate server restart" patterns, or a consumer that
|
||||
// explicitly re-instantiates without closing first). This isn't the
|
||||
// dangerous cross-process case the lock exists to prevent — the two
|
||||
// instances share a memory space and can't silently diverge from each
|
||||
// other beyond what their callers already see. Warn and take over.
|
||||
if (existing.pid === myPid && existing.hostname === hostname && !options?.force) {
|
||||
console.warn(
|
||||
`[brainy] Re-acquiring writer lock for ${this.rootDir} held by the same process (PID ${existing.pid}). ` +
|
||||
`If you intended to keep the previous Brainy instance alive, this is a bug — close it first.`
|
||||
)
|
||||
const info: WriterLockInfo = {
|
||||
pid: myPid,
|
||||
hostname,
|
||||
startedAt: now,
|
||||
lastHeartbeat: now,
|
||||
version: getBrainyVersion(),
|
||||
rootDir: this.rootDir
|
||||
}
|
||||
await this.writeFileAtomic(lockFile, JSON.stringify(info, null, 2))
|
||||
this.installWriterLock(info)
|
||||
return info
|
||||
}
|
||||
|
||||
const stale = !options?.force && (await this.isWriterLockStale(existing))
|
||||
if (!options?.force && !stale) {
|
||||
// Consumer-facing error contract: callers detect this case via
|
||||
// err.code and read the holder's details from err.lockInfo.
|
||||
const err = new Error(
|
||||
`Another writer holds this Brainy directory.\n` +
|
||||
` PID: ${existing.pid} on host ${existing.hostname}\n` +
|
||||
` Started: ${existing.startedAt}\n` +
|
||||
` Heartbeat: ${existing.lastHeartbeat}\n` +
|
||||
` Version: ${existing.version}\n` +
|
||||
` Directory: ${this.rootDir}\n\n` +
|
||||
`For diagnostic queries against this live store, use:\n` +
|
||||
` const reader = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: '${this.rootDir}' } })\n\n` +
|
||||
`If you have verified the existing lock is stale (e.g. a crashed writer on a different host that PID liveness cannot reach), pass { force: true }.`
|
||||
) as Error & { code: string; lockInfo: WriterLockInfo }
|
||||
err.code = 'BRAINY_WRITER_LOCKED'
|
||||
err.lockInfo = existing
|
||||
throw err
|
||||
throw this.writerLockedError(existing)
|
||||
}
|
||||
|
||||
console.warn(
|
||||
`[brainy] Overwriting stale writer lock for ${this.rootDir} ` +
|
||||
`(PID ${existing.pid} on ${existing.hostname} appears dead).`
|
||||
options?.force
|
||||
? `[brainy] Force-overwriting writer lock for ${this.rootDir} ` +
|
||||
`(was held by PID ${existing.pid} on ${existing.hostname}).`
|
||||
: `[brainy] Overwriting stale writer lock for ${this.rootDir} ` +
|
||||
`(PID ${existing.pid} on ${existing.hostname} appears dead).`
|
||||
)
|
||||
// Takeover: verify the file still holds the lock we judged (a live
|
||||
// successor may have claimed meanwhile), then remove it and fall
|
||||
// through to the atomic claim below. A racing claimer who beats us to
|
||||
// the create simply wins — our next loop iteration reads their fresh
|
||||
// lock and throws honestly. (Advisory file locking has no
|
||||
// compare-and-delete; staleness requiring a 60s-old heartbeat keeps
|
||||
// the residual verify-to-unlink window practically unreachable.)
|
||||
const recheck = await this.readWriterLock()
|
||||
if (
|
||||
recheck &&
|
||||
(recheck.pid !== existing.pid ||
|
||||
recheck.startedAt !== existing.startedAt ||
|
||||
recheck.lastHeartbeat !== existing.lastHeartbeat)
|
||||
) {
|
||||
continue // the lock changed hands while we deliberated — re-evaluate
|
||||
}
|
||||
try {
|
||||
await fs.promises.unlink(lockFile)
|
||||
} catch (err: any) {
|
||||
if (err.code !== 'ENOENT') throw err
|
||||
}
|
||||
}
|
||||
} else if (existing && options?.force) {
|
||||
console.warn(
|
||||
`[brainy] Force-overwriting writer lock for ${this.rootDir} ` +
|
||||
`(was held by PID ${existing.pid} on ${existing.hostname}).`
|
||||
)
|
||||
|
||||
const info: WriterLockInfo = {
|
||||
pid: myPid,
|
||||
hostname,
|
||||
startedAt: existing && options?.force ? existing.startedAt : now,
|
||||
lastHeartbeat: now,
|
||||
version: getBrainyVersion(),
|
||||
rootDir: this.rootDir
|
||||
}
|
||||
|
||||
// The atomic claim: create-exclusive, so exactly ONE racer wins.
|
||||
try {
|
||||
await fs.promises.writeFile(lockFile, JSON.stringify(info, null, 2), { flag: 'wx' })
|
||||
} catch (err: any) {
|
||||
if (err.code === 'EEXIST') {
|
||||
continue // someone else claimed between our read and create — re-evaluate
|
||||
}
|
||||
throw err
|
||||
}
|
||||
|
||||
this.installWriterLock(info)
|
||||
return info
|
||||
}
|
||||
|
||||
const info: WriterLockInfo = {
|
||||
pid: typeof process !== 'undefined' && process.pid ? process.pid : 0,
|
||||
hostname,
|
||||
startedAt: existing && options?.force ? existing.startedAt : now,
|
||||
lastHeartbeat: now,
|
||||
version: getBrainyVersion(),
|
||||
rootDir: this.rootDir
|
||||
}
|
||||
// Attempts exhausted: something is claiming this directory faster than we
|
||||
// can evaluate it. Read whoever holds it now and fail loudly with their
|
||||
// details rather than degrading into a lockless open.
|
||||
const holder = await this.readWriterLock()
|
||||
if (holder) throw this.writerLockedError(holder)
|
||||
throw new Error(
|
||||
`Failed to acquire the writer lock for ${this.rootDir} after ${MAX_ATTEMPTS} attempts — ` +
|
||||
`the lock file is being contended. Retry, or inspect ${lockFile}.`
|
||||
)
|
||||
}
|
||||
|
||||
await this.writeFileAtomic(lockFile, JSON.stringify(info, null, 2))
|
||||
/** Record lock ownership + start the unref'd heartbeat. */
|
||||
private installWriterLock(info: WriterLockInfo): void {
|
||||
this.writerLockInfo = info
|
||||
|
||||
// Heartbeat — rewrite lastHeartbeat every WRITER_HEARTBEAT_MS so other
|
||||
|
|
@ -1835,8 +1888,24 @@ export class FileSystemStorage extends BaseStorage {
|
|||
// Don't keep the event loop alive just for the heartbeat.
|
||||
this.writerLockHeartbeat.unref()
|
||||
}
|
||||
}
|
||||
|
||||
return info
|
||||
/** The consumer-facing BRAINY_WRITER_LOCKED error, holder details attached. */
|
||||
private writerLockedError(existing: WriterLockInfo): Error {
|
||||
const err = new Error(
|
||||
`Another writer holds this Brainy directory.\n` +
|
||||
` PID: ${existing.pid} on host ${existing.hostname}\n` +
|
||||
` Started: ${existing.startedAt}\n` +
|
||||
` Heartbeat: ${existing.lastHeartbeat}\n` +
|
||||
` Version: ${existing.version}\n` +
|
||||
` Directory: ${this.rootDir}\n\n` +
|
||||
`For diagnostic queries against this live store, use:\n` +
|
||||
` const reader = await Brainy.openReadOnly({ storage: { type: 'filesystem', path: '${this.rootDir}' } })\n\n` +
|
||||
`If you have verified the existing lock is stale (e.g. a crashed writer on a different host that PID liveness cannot reach), pass { force: true }.`
|
||||
) as Error & { code: string; lockInfo: WriterLockInfo }
|
||||
err.code = 'BRAINY_WRITER_LOCKED'
|
||||
err.lockInfo = existing
|
||||
return err
|
||||
}
|
||||
|
||||
public override async releaseWriterLock(): Promise<void> {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue