Skip to content

src: run native immediates during Environment cleanup

This is a fix for #30643 (closed) that is, unfortunately, a bit more complex than I’d like. The last commit message provides an alternative, so if anyone prefers then I’d probably implement that first as quick fix for the flaky test and then rebase this PR against it.


src: keep object alive in stream_pipe code

This was overlooked in a489583e.

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

src: add more can_call_into_js() guards

This is in preparation for running native SetImmediate() callbacks during shutdown.

src: no SetImmediate from destructor in stream_pipe code

Guard against running SetImmediate() from the destructor. The object will not be alive or usable in the callback, so it does not make sense to attempt to schedule the SetImmediate().

src: run native immediates during Environment cleanup

This can be necessary, because some parts of the Node.js code base perform cleanup operations in the Immediate callbacks, e.g. HTTP/2.

This resolves flakiness in an HTTP/2 test that failed when a SetImmediate() callback was not run or destroyed before the Environment destructor started, because that callback held a strong reference to the Http2Session object and the expectation was that no such objects exist once the Environment constructor starts.

Another, slightly more direct, alternative would have been to clear the immediate queue rather than to run it. However, this approach seems to make more sense as code generally assumes that the SetImmediate() callback will always run; For example, N-API uses an immediate callback to call buffer finalization callbacks.

Unref’ed immediates are skipped, as the expectation is generally that they may not run anyway.

Fixes: https://github.com/nodejs/node/issues/30643

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

Merge request reports

Loading