move away from process.nextTick

[ronag]

There are two problems with process.nextTick.

1. (LOW SEVERITY) It reduces performance for all async callbacks from native code.

2. (HIGH SEVERITY) It causes weird and unpredictable behavior when trying to interop with promises and async/await code.

In particular, we have an invariant where we always emit events and invoke callbacks "asynchronously". However, that doesn't apply to Promise, since we "force" asynchronously through process.nextTick which occurs before any microtick. Hence, for any promise/micro-tick-based code things appear to occur synchronously. This leads to all kinds of weird and hard-to-debug bugs.

e.g.

const foo = await new Promise((resolve) => resolve(new Bar()))
// You can replace `'error'` with any event name, `'open'`, `'close'`, etc...
// event might be emitted through process.nextTick before 
// we ever  get a chance to install a handler.
foo.on('error', errorHandler) a handler.

Refs: #51070
Refs: #51156

[nodejs-github-bot]

Review requested:

• @nodejs/crypto
• @nodejs/http
• @nodejs/net
• @nodejs/startup

[ronag]

This should probably be split into separate PRs for each module with tests added. However, I don't have time for that.

I'm mostly opening this PR for visibility and discussion and hoping someone might take over if consensus is that a simple find and replace PR is not landable.

[mcollina]

this needs a Citgm run and investigate any failures deeply, I have a feeling this will be breaking.

I’m +1 in principle.

[ronag]

I've done what I have time with. This needs another champion to get over finish line.

[NiharPhansalkar]

Hello @ronag. I have understood the issue here, but I am not sure I understand the solution to this problem. Can you point me in the right direction? I did search around and ask AI, and it seems the suggested solution is to replace process.nextTick() with queueMicrotask?
Can you confirm if this is exactly what has to be done in order to resolve this issue? Or are there any other changes that are required?

[ronag]

Related #51156

[jasnell]

Like @mcollina I'm +1 with the idea here but I definitely think this needs to be very carefully tested as this could end up breaking a LOT of code out there.

I also wonder if we shouldn't give users a temporary escape hatch even after this moves forward to allow them to opt-back-in to using process.nextTick instead. That can be accomplished by setting up an internal alias method that by default is queueMicrotask but is set to process.nextTick if a CLI flag or env var is used.

e.g.

const internalTick = hasOption('--use-internal-nexttick') ? process.nextTick : queueMicrotask;

[ShogunPanda]

I reviewed the files.
The changes looks fine to me, but definitely like James' idea. I think it's the safest approach here, for both users and us.

[jasnell]

Another option here: Perhaps instead of changing the process.nextTick callsites into queueMicrotask calls directly... perhaps process.nextTick itself could become an alias for queueMicrotask. This would help to avoid odd timing cases where Node.js changes to queueMicrotask but user code still contains to use process.nextTick

[Qard]

Yeah, I would go for just swapping out the internals of process.nextTick to delegate to queueMicrotask and then include a flag as an escape hatch to restore the old behaviour in case it breaks anyone. They'll no doubt come to the issue tracker and complain, which will give us a signal on impact, but having an immediate fix for them with a flag should soften the blow. 😅