Skip to content

Stop the worker pool respawning workers that can't start - #353

Merged
slimbuck merged 5 commits into
playcanvas:mainfrom
slimbuck:worker-dev
Oct 9, 2026
Merged

slimbuck merged 5 commits into
playcanvas:mainfrom
slimbuck:worker-dev

Conversation

@slimbuck

@slimbuck slimbuck commented Oct 8, 2026

Copy link
Copy Markdown
Member

When workers can't load (e.g. a worker script served with a non-JavaScript MIME type, a CSP, or a network failure), the pool only fell back to inline once no slots were left. With several tasks queued it starts several workers, so each failure left others still starting, and the pool replaced the failed one, forever: thousands of worker starts a second, with the queued tasks never running.

Before any worker has been ready, a start failure now isn't replaced; the pool waits for the workers still starting and goes inline when the last one fails. Adds a test against the built library, which points the pool at a worker that throws on load.

When workers can't load (e.g. a worker script served with a non-JavaScript MIME type, a CSP, or a network failure), the pool only fell back to inline once no slots were left. With several tasks queued it starts several workers, so each failure left others still starting, and the pool replaced the failed one, forever: thousands of worker starts a second, with the queued tasks never running.

Before any worker has been ready, a start failure now isn't replaced; the pool waits for the workers still starting and goes inline when the last one fails. Adds a test against the built library, which points the pool at a worker that throws on load.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@slimbuck
slimbuck requested a balanced review from Copilot October 8, 2026 16:58
@slimbuck slimbuck self-assigned this Oct 8, 2026
@slimbuck slimbuck added the bugfix Something isn't working label Oct 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Later startup failures can still trigger an unbounded respawn loop after any worker has previously become ready.

1 open finding
What changed in this PR

Prevents endless worker respawning during initial startup failures and adds bundled-library regression coverage.

Changes:

  • Defers inline fallback until all starting workers fail.
  • Tests failed worker startup with queued tasks.
File Description
src/​lib/​workers/​worker-queue.ts Adjusts worker startup-failure handling.
test/​worker-queue-bundled.test.mjs Adds bundled worker failure regression test.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/lib/workers/worker-queue.ts Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Healthy workers can still trigger repeated replacement attempts for failed startup slots while work remains queued.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/lib/workers/worker-queue.ts
@slimbuck

slimbuck commented Oct 8, 2026

Copy link
Copy Markdown
Member Author

/deploy

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Synchronous browser worker failures can bypass the latch within the active spawn loop.

1 open finding
1 resolved since last review

🧠 Review effort: Balanced

Comment thread src/lib/workers/worker-queue.ts
@slimbuck
slimbuck requested a review from a team October 9, 2026 09:10
@slimbuck
slimbuck merged commit b733a9e into playcanvas:main Oct 9, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants