Skip to content

test_runner: do not reuse a worker ID held by a running file - #65739

Open
vserpokryl wants to merge 1 commit into
nodejs:mainfrom
vserpokryl:test-runner-exclusive-worker-id
Open

test_runner: do not reuse a worker ID held by a running file#65739
vserpokryl wants to merge 1 commit into
nodejs:mainfrom
vserpokryl:test-runner-exclusive-worker-id

Conversation

@vserpokryl

Copy link
Copy Markdown

WorkerIdPool handed out worker IDs round-robin ((nextId++ % maxConcurrency) + 1)
and never released them, so an ID was only unique among the first N test files.
As soon as files finished out of order, a file that started later was given an ID
still held by a live process, which defeats the purpose of context.workerId:
allocating a database, port or directory per worker.

With --test-concurrency=2 and three files, where the first one outlives the
second:

start slow 1
start fast 2
end fast 2
start last 1   <-- same ID as the still-running 'slow'
end last 1
end slow 1

Track the IDs that are actually in use and hand out the lowest free one, then
release it once the child process is gone. The release happens in a finally
block so an aborted run or a failed spawn does not leak the ID out of the pool.

The pool no longer needs to be told the concurrency level. That also removes a
mismatch: the pool was sized from globalOptions.concurrency ?? concurrency,
which can differ from the concurrency the root test actually enforces. Since IDs
are handed out lowest-first, the highest ID in use is now bounded by how many
files really run at once.

Refs: #61394

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels Sep 2, 2026
Worker IDs were handed out round-robin and never released, so a file
that started after another finished could get an ID still held by a
live process. Track the IDs in use, hand out the lowest free one, and
release it in a finally block once the child process exits.

Refs: nodejs#61394
Signed-off-by: Vasiliy Serpokryl <vasiliy.serpokryl@mail.ru>
@vserpokryl
vserpokryl force-pushed the test-runner-exclusive-worker-id branch from 35a00d4 to 5416f6a Compare September 2, 2026 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants