Skip to content

worker: move JoinThread() back into exit callback

Rodrigo Muino Tomonari requested to merge github/fork/addaleax/worker-exit-fix into master
worker: move JoinThread() back into exit callback

de2c68c7 moved this call to the destructor, under the assumption that that would essentially be equivalent to running it as part of the callback since the worker would be destroyed along with the callback.

However, the actual code in Environment::RunAndClearNativeImmediates() comes with the subtlety that testing whether a JS exception has been thrown happens between the invocation of the callback and its destruction, leaving a possible exception from JoinThread() potentially unhandled (and unintentionally silenced through the TryCatch).

This affected exceptions thrown from the 'exit' event of the Worker, and made the parallel/test-worker-message-type-unknown test flaky, as the invalid message was sometimes only received during the Worker thread’s exit handler.

Fix this by moving the JoinThread() call back to where it was before.

Refs: https://github.com/nodejs/node/pull/31386

src: harden running native SetImmediate()s slightly

Prevent mistakes like the one fixed by the previous commit by destroying the callback immediately after it has been called.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Merge request reports