fix: call node::Stop on exit

[nornagon]

Description of Change

I believe this fixes #25267, the "illegal access" message we sometimes see at exit.

I don't have a test or a full root cause analysis for this yet, but what appears to be happening is this:

1. When we call node::FreeEnvironment from ElectronBrowserMainParts::PostMainMessageLoopRun, it calls node::CleanupHandles.
2. node::CleanupHandles disallows JS execution, using Isolate::DisallowJavascriptExecutionScope.
3. node::CleanupHandles triggers a uv loop run, which triggers ElectronBindings::OnCallNextTick.
4. ElectronBindings::OnCallNextTick runs the microtask queue, which sometimes includes an entry into JS. (Not always, though. This is the bit I'm confused about: what are the conditions in which there's a pending JS microtask here?)
5. Attempting to execute JS while JS execution is disallowed throws an "illegal access" exception.

This change causes env->can_call_into_js() to return false after node::Stop() is called, which means InternalCallbackScope::Close() won't run any JS, which avoids this error.

The remaining questions in my mind are:

1. Why were we not calling node::Stop() before? Was it simply an oversight, or was it an intentional choice?
2. Under what circumstances does CleanupHandles end up invoking JS? What is putting a JS task in the microtask queue during exit?
3. How can we write a test for this?

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• tests are changed or added
• PR title follows semantic commit guidelines
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: Fixed an issue that could cause a normally-exiting process to fail with an "illegal access" message and exit code 7.

[nornagon]

cc @addaleax @codebytere

[zcbenz]

• Why were we not calling node::Stop() before? Was it simply an oversight, or was it an intentional choice?

node::Stop() was only introduced in v11 and no one had noticed this new handy API since then.

[codebytere]

We'll also want to invoke this in ELECTRON_RUN_AS_NODE (e.g in node_main) @nornagon

with regard to why we weren't previously calling it, @zcbenz is correct, it wasn't introduced until v11 and was just an oversight. I'll try to spend some time thinking about a potential way to test this more directly 🤔

[nornagon]

The node embedder teardown API certainly seems a lot more complex than it needs to be 🤔

[release-clerk]

Release Notes Persisted

Fixed an issue that could cause a normally-exiting process to fail with an "illegal access" message and exit code 7.

[trop]

I have automatically backported this PR to "10-x-y", please check out #25458

[trop]

I have automatically backported this PR to "11-x-y", please check out #25459

[shawnchain]

It seems we encountered the same issue on 8.4.1. any hints to port this patch to 8.x.y branch ?

[nornagon]

Yeah, if this was added in Node 11 then we should probably backport to 8.x.

[nornagon]

/trop run backport-to 9-x-y 8-x-y

[trop]

The backport process for this PR has been manually initiated -
sending your commits to "9-x-y"!

[nornagon]

/trop run backport-to 8-x-y

[trop]

I was unable to backport this PR to "9-x-y" cleanly;
you will need to perform this backport manually.

[trop]

The backport process for this PR has been manually initiated -
sending your commits to "8-x-y"!

[trop]

I was unable to backport this PR to "8-x-y" cleanly;
you will need to perform this backport manually.

[trop]

@nornagon has manually backported this PR to "8-x-y", please check out #25501

[trop]

@nornagon has manually backported this PR to "9-x-y", please check out #25502

[Raynos]

When will this be release in 10.1.6 ?