Skip to content

fix: treat null signal as no cancellation under contention - #24

Merged
Kikobeats merged 2 commits into
masterfrom
cursor/critical-bug-management-1837
Aug 5, 2026
Merged

fix: treat null signal as no cancellation under contention#24
Kikobeats merged 2 commits into
masterfrom
cursor/critical-bug-management-1837

Conversation

@cursor

@cursor cursor Bot commented Jul 25, 2026

Copy link
Copy Markdown
Contributor

Bug and impact

Under contention, lock(null) / withLock(fn, null) rejected with TypeError after enqueueing a waiter. That zombie then made the holder's release() throw on null.removeEventListener, so:

  1. Successful withLock work was reported as failed (throw in finally)
  2. Later acquirers could jump ahead of live FIFO waiters while isLocked() briefly looked free

Root cause

The abortable path used signal !== undefined, so null entered abort setup. addEventListener threw after enqueue, leaving a waiter whose patched acquire() also assumed a real signal.

Fix

  • Treat null like "no signal" (signal != null)
  • If subscribe throws for a non-signal value, unlink the waiter before rethrowing

Validation

  • Added regression tests for null under contention, withLock holder release, and non-signal cleanup
  • npm test: 20/20 passed at 100% coverage
Open in Web View Automation 

Note

Low Risk
Small, targeted change to optional AbortSignal handling in the lock implementation, with new regression tests and no API surface change.

Overview
Fixes a contention bug where lock(null) / withLock(fn, null) incorrectly entered abort handling (signal !== undefined), which could enqueue a broken waiter, throw on addEventListener, and later break the holder’s release() (FIFO and error reporting).

create.js now treats null like “no signal” via signal != null for abort wiring, and rejects non-AbortSignal values up front with TypeError before queueing or taking a slot.

Regression coverage in test/cancel.js for null under contention, withLock holder release, and invalid signal arguments.

Reviewed by Cursor Bugbot for commit 004bbae. Bugbot is set up for automated code reviews on this repo. Configure here.

cursoragent and others added 2 commits July 25, 2026 04:11
Passing null as the AbortSignal under contention took the abortable
queue path (signal !== undefined), so addEventListener threw after the
waiter was enqueued. That left a zombie whose acquire() also threw on
release, rejecting successful withLock work and letting later acquirers
jump the FIFO queue.

Treat null like undefined, and unlink the waiter if subscribe throws.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MTbj6CYXGsk6rhXx7J8SUP
@Kikobeats
Kikobeats marked this pull request as ready for review August 5, 2026 09:11
@Kikobeats
Kikobeats merged commit 03f66a4 into master Aug 5, 2026
3 checks passed
@Kikobeats
Kikobeats deleted the cursor/critical-bug-management-1837 branch August 5, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants