Skip to content

test(writer-lock): hold the lock with a live child, not a pid assumed alive - #1753

Open
bompus wants to merge 1 commit into
colbymchenry:mainfrom
bompus:fix/writer-lock-live-holder
Open

test(writer-lock): hold the lock with a live child, not a pid assumed alive#1753
bompus wants to merge 1 commit into
colbymchenry:mainfrom
bompus:fix/writer-lock-live-holder

Conversation

@bompus

@bompus bompus commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Fixes #1752.

The problem

writer lock (#1740) > reports taken when a live foreign pid holds the lock fails on Windows, on a clean main, with nothing else present:

AssertionError: expected 'acquired' to be 'taken'
  Tests  1 failed | 3 passed (4)

The fixture writes pid: 1 as its live foreign holder and says so in its own comment — "On Linux, PID 1 is almost always alive." Windows has no PID 1, so tryAcquireWriterLock reads the holder as dead, clears the stale lock and acquires. That is the correct behaviour for a dead holder; the assertion is failing on the fixture, not on the lock.

The change

Spawn a parked child and use its pid. It is alive on every platform, and "does not steal from a live holder" is what the lock actually promises — so the case now tests the promise rather than a number that happens to name a running process. On Linux it also stops depending on the ambient assumption about init. The child is killed in afterEach.

The sibling clears a stale dead-pid lock and acquires case still covers the dead-holder direction with 2147483646, so both sides of the branch stay tested.

Verification

  • Windows 11, Node 26.8.1, on this branch: __tests__/writer-lock.test.ts 4/4 pass
  • Same file on bb204f8 without the change: 1 failed / 3 passed, the case above

I have not been able to run the Linux arm; the change is platform-neutral by construction, but worth a CI look.

Why I noticed

A downstream promotion pipeline gates on a zero-failure suite, so this single test blocked a Windows host from promoting any build of main. Not urgent for the lock itself, which behaves correctly throughout.

… alive

The live-holder case wrote `pid: 1` and leaned on its own comment, "On Linux,
PID 1 is almost always alive". Windows has no PID 1, so tryAcquireWriterLock
read the holder as dead and acquired — the correct behaviour for a dead
holder, and a failing assertion. The platform-specific part was the fixture,
not the lock.

A parked child process is alive on every platform, and not stealing from a
live holder is what the lock actually promises, so the case now tests the
promise rather than a number that happens to name a running process. The child
is reaped in afterEach.

Fixes colbymchenry#1752.
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.

writer-lock test assumes a live PID 1, so #1744 cannot pass on Windows

1 participant