Skip to content

refactor: remove use of PThread.runningWorkers#216

Open
toyobayashi wants to merge 1 commit into
mainfrom
remove-runningWorkers
Open

refactor: remove use of PThread.runningWorkers#216
toyobayashi wants to merge 1 commit into
mainfrom
remove-runningWorkers

Conversation

@toyobayashi
Copy link
Copy Markdown
Owner

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Removes the now-redundant PThread.runningWorkers array (deprecated upstream in emscripten-core/emscripten#26998) and derives running workers from PThread.pthreads instead. Also tightens the cleanup guard in WASIThreads spawn error handling.

Changes:

  • Delete runningWorkers field from ThreadManager and all push/splice maintenance sites.
  • Replace runningWorkers.forEach usages with Object.values(PThread.pthreads).forEach in TSFN, async-work, and the associated @__postset comment.
  • Consolidate cleanThread calls in WASIThreads spawn into a single catch block guarded by worker !== undefined && tid !== undefined.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/wasi-threads/src/thread-manager.ts Remove runningWorkers field/maintenance; iterate pthreads in terminateAllThreads.
packages/wasi-threads/src/wasi-threads.ts Make tid optional and move cleanup into the catch block with guards.
packages/emnapi/src/threadsafe-function.ts Replace runningWorkers.forEach with Object.values(PThread.pthreads).forEach.
packages/emnapi/src/core/async-work.ts Same replacement for the async-work listener setup.
packages/emnapi/src/emscripten/async.ts Update @__postset doc comment to reflect the new iteration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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