Possible memory leak-- `processPromiseRejections ` never called?

[mattkrick]

After migrating from express to uwebsockets, i noticed a memory leak caused by node holding onto unhandled rejections and never flushing them

The pendingUnhandledRejections array is emptied when processPromiseRejections is called: https://github.com/nodejs/node/blob/c9b93e234454322ac0b7a6cd29d394f428f3e37d/lib/internal/process/promises.js#L171

It should get called every tick here: https://github.com/nodejs/node/blob/68abaab8baac203833889c9106abf6fe82a5900f/lib/internal/process/task_queues.js#L98

Perhaps the task queue is never empty, so !queue.isEmpty() || processPromiseRejections() always shortcircuits? Any guidance appreciated

That's probably a correct find, I've been spending far too long trying to fight that whole dreadful nextTick nonsense.

It has been the finger nail in my eye for a long time and I properly hate it for existing.

The only proper way is to use node::MakeCallback which is a massive performance hit since nodejs is a pile of garbage.

In any case, I'll probably have to accept nodejs is nodejs. It's easier to deal with knowing Python has no such nonsense.

[mattkrick]

i hear ya,
dumb idea because i'm not familiar with node internals-- is it possible to just call process.runNextTicks?
Since _tickCallback was deprecated, that looks like the new userland way to handle it.

issue: nodejs/node#29671
old example from node v10: https://blog.insiderattack.net/crossing-the-js-c-boundary-advanced-nodejs-internals-part-1-cb52957758d8

It's an easy fix, all you have to do is resort to the "correct" way of calling in to JavaScript.

Problem is, that "correct" way is really bad, and limits performance a lot.

Vanilla V8 Function calls are fast, I get 123k messages/second while node::MakeCallback is down to 99k. It's a major hit to performance.

However, since I have found Python and already do 137k messages/second there I think it is time to just let nodejs be nodejs like I said earlier, and work more on the Python extension when it comes to showcasing performance.

Moving back to the "correct" way would solve a lot of strange crashes when debugging and just overall make it more robust.

I see it as a speeding ticket, now it's time to pay the price.

[IhsanMujdeci]

Does this mean you will stop developing the js library?

No I'm just saying, writing the Python extension is a pleasure. It's the scripting environment that really lets uWS shine

@mattkrick Please try npm install uNetworking/uWebSockets.js#binaries and see if that solves your problems

[mtsewrs]

@alexhultman why do this if performance is sacrificed? Is this a non issue if you just catch all promises?

Because there are other problems such as the debugger crashing.

[mattkrick]

verified that the leak no longer exists after installing binaries. thank you!

sidenote-- i tried to downgrade to v16.5.0 to reproduce the leak, but kept getting an error during heap dump (/usr/local/bin/node[6355]: ../src/api/callback.cc:110:void node::InternalCallbackScope::Close(): Assertion (env_->execution_async_id()) == (0)' failed.`). blew away the yarn cache, node_modules & lockfile, still couldn't get the heapdump to work for the old version. not a huge deal, just stumped on what could've broken

That crash you got in v16.5 is exactly the kind of issues this commit solves, so it further highlights the need for it