Bound the exclusive 'update-attributes' lock wait and release it structurally

[kriszyp]

Fixes #2251.

The exclusive update-attributes lock was acquired at two sites with an unbounded busy-spin (while (!rootStore.tryLock('update-attributes')) {}) — no deadline, no yield, and the key string re-marshalled on every attempt. A holder that never released (e.g. a crashed schema operation) pinned the spinning worker's core at 100% forever, silently; the observed incident burned ~8.8 cores for 16–18 hours. Release was also hand-enforced at each escape path in table() (three ad-hoc calls, only one in a finally), so every early return or throw between acquire and release was a fresh chance to wedge a core — one such leak had already been hand-patched (the deleted catch documented it).

What changed:

• Bounded acquire — new shared acquireUpdateAttributesLock in Table.ts: fast-path tryLock, ~2ms hot spin (brief contention keeps today's latency; a release by another thread is observable without yielding), then bounded Atomics.wait sleeps (1→16ms backoff), then ServerError at the shared 10s LOCK_TIMEOUT (the existing constant, now exported) with the lock name, the database.table in scope, and the elapsed time in the message.
• Structural release — table() wraps every path that can acquire in a single try/finally with an idempotent releaseLock() (idempotent because the created-concurrently early return must release before its recursive reload — the lock is not reentrant — and a bare second unlock could release another thread's lock). The three hand-placed releases are subsumed; the release point is unchanged, so lock hold time does not grow. dropTable's locked section now runs under a withUpdateAttributesLock(fn) wrapper.
• Hoisted key marshalling — the lock key is pre-encoded once at module level (Buffer.from('update-attributes'), byte-identical to ordered-binary's string encoding, verified at runtime by the tests).
• The LMDB branch of exclusiveLock() is untouched; the structural release covers it identically to before.
• DESIGN.md's spin-lock trap paragraph updated to the bounded/structural reality.

For the human reviewer

• Fixed 10s deadline, no holder-liveness check (the review's major finding): a legitimately long critical section (e.g. completeInterruptedDrop over many column families on saturated I/O) can exceed 10s, making waiters throw where the old spin eventually converged. This is the specified contract for this fix (bound + throw); a holder-liveness stamp is a possible future refinement if spurious timeouts appear under load.
• Timeout statusCode is 500 (ServerError default). A retryable 503 (the IndexRebuildingError precedent) would tell replication/clients to retry transient contention; trivial to change if preferred.
• table() mutates the live Table metadata before the lock is taken (pre-existing shape): a deadline throw can leave this worker's in-memory schema ahead of the catalog until reload. Pre-dates this change; the deadline adds a new trigger. Left as-is deliberately — restructuring the mutation order is a larger change.
• Wide try/finally vs extracting table()'s locked region into the withUpdateAttributesLock callback form: the lock is acquired lazily at five conditional points mid-flow, so the callback form would force eager acquisition (a contention regression); the wide guard preserves acquisition semantics exactly.
• Wall-clock deadline (Date.now()): an NTP step backwards extends one DDL wait; repo-wide convention, previously infinite.

Verification

• Six new unit tests (unitTests/resources/updateAttributesLock.test.js + a worker-thread holder): uncontended pass-through, helper deadline throw (parameterized 250ms) with lock/table/elapsed in the message, release-on-callback-throw, table()-level structural release (injected catalog failure mid-schema-update → throw → lock free → subsequent schema op succeeds), a real table() create against a wedged lock throwing ServerError at ~10.0s and succeeding after unlock, and cross-thread brief contention (worker holds via the string key, main acquires via the Buffer key — pinning the key-encoding equivalence).
• Fails-on-base proven with a full rebuild against origin/main sources: the structural-release test fails exactly on "exclusive lock must not leak when table() throws", and the production-path deadline test hangs in the unbounded spin (killed by an external 45s timer) — the defect itself.
• test:unit:resources (RocksDB and LMDB engines) and lint/format/typecheck clean; the only local failures (randomAccessFields ×2, replayStructures, and a test:unit:main load crash) reproduce identically on pristine origin/main on this machine (stale local test roots/config) and are unrelated.
• End-to-end route: not observable end-to-end short of wedging a live worker; the real-table() deadline test is the closest executable proof of the production path.
• Cross-model pre-push review: round 1 full (codex graded + gemini + cursor-composer + Harper-domain adjudication), round 2 delta (codex + gemini) — no new findings in round 2; everything actionable was fixed (production-path test, comment sweep, DESIGN.md), remaining items are the judgment calls listed above.

Complexity: moderate — concurrency-sensitive locking change on the schema/DDL path, but no data-format or hot-path changes and the critical-section boundaries are preserved.

— Claude Fable

🤖 Generated with Claude Code

_{Review-Coverage: authored=unknown; ran=none; rounds=1 @ c329356}

_{Human-Review-Need: 4 @ c329356}

[gemini-code-assist]

Code Review

This pull request replaces the unbounded synchronous spin lock for the exclusive update-attributes lock with a synchronous bounded wait mechanism using Atomics.wait and a 10-second timeout to prevent worker cores from pinning at 100% CPU when a lock is leaked. It introduces helper functions acquireUpdateAttributesLock, releaseUpdateAttributesLock, and withUpdateAttributesLock in Table.ts, refactors makeTable and table() to use these helpers, and adds comprehensive unit tests. The reviewer suggested using assert.strictEqual instead of assert.equal in the unit tests to align with the repository style guide.

[claude]

Reviewed; no blockers found.