electron: [Bug]: Uncaught illegal state exception and renderer crash with node immediates and closing child window

Preflight Checklist

• I have read the Contributing Guidelines for this project.
• I agree to follow the Code of Conduct that this project adheres to.
• I have searched the issue tracker for a bug report that matches the one I want to file, without success.

Electron Version

13.x, 17.x, 22.0.0

What operating system are you using?

Windows

Operating System Version

Windows 11 (22621.963)

What arch are you using?

x64

Last Known Working Electron version

Unknown

Expected Behavior

Closing a child window should not produce an ‘Uncaught illegal state’ exception. Furthermore, it shouldn’t cause the renderer process to exit with code 7 (which can specifically occur if there is a node native immediate being used, see additional information below).

Actual Behavior

A race condition can occur when using setImmediate (or using a node module that sets up native immediates) in a node environment and closing a child window that has a separate node environment (but shares the underlying isolate and event loop).

Note: This seems to occur when the BrowserWindow webPreferences sandbox is set to false.

The behavior here is that the console will report Uncaught illegal state and in certain cases the renderer process will simply exit with code 7. The following was also reported to stderr:

illegal access
(Use `electron --trace-uncaught ...` to show where the exception was thrown)

Testcase Gist URL

https://gist.github.com/markh-discord/1bb303c57d1a20aeee95aeedbc403ab3

Additional Information

Note: This seems to occur only when the main BrowserWindow webPreferences sandbox is set to false.

The reproduce project in the gist may require a few attempts, but when clicking the button to open the window it will automatically close. Around 1-10 attempts may be needed to display the error in the console. To improve the simplicity of this I used Javascript setImmediate which seems to be more recoverable as it doesn’t exit the process. In the steps below there is a node native immediate being setup by the node https module that does cause the renderer process to eventually exit.

Reproduce steps aren’t precise since this is a race condition, however the following path seems to hit this:

1. node Environment A is created and Environment::InitializeLibuv is run and sets up its Environment::CheckImmediate method in uv_check_start. a. This will be interesting later, but it’s called via the event loop uv_run: https://github.com/libuv/libuv/blob/e972c6705f197e12d211b598fc514bbea051425d/src/win/core.c#L645
2. Window is opened, a new node Environment (I’ll call Environment B) is created: https://github.com/electron/electron/blob/dfe501941cd9e7092829c3ef8e27e322047cf736/shell/renderer/electron_renderer_client.cc#L91-L92 a. This environment uses the existing NodeBindings::isolate_data_, which also shares the isolate and uv_loop_ from Environment A: https://github.com/electron/electron/blob/dfe501941cd9e7092829c3ef8e27e322047cf736/shell/common/node_bindings.cc#L493
3. node Environment A has a native immediate pushed from http post work being done by the node http module. a. This can occur in a few places via env()->SetImmediate(...), here’s one as an example: https://github.com/nodejs/node/blob/e8db11c5b2169931238db6c02f6d7f80b3c6e759/src/crypto/crypto_tls.cc#L593
4. Window is closed, causing Environment B to begin cleanup: a. node::FreeEnvironment is called, and sets up JS to be disallowed: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/api/environment.cc#L420-L421 b. node::CleanupHandles is eventually called, this starts up the event loop via uv_run: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/env.cc#LL966C21-L966C21 c. Per 1a above, the uv_check_invoke is called, and the Environment A’s CheckImmediate is run
5. At this point we’re now running things from Environment A, in particular it will begin executing the native immediates that it has. Note that we’re still in the same call stack and the isolate’s state to disallow JS is still flagged.
6. Since a native immediate was added in step 2 above, this begins running, eventually for the http stream work it tries to callback into Javascript via node::ReportWritesToJSStreamListener::OnStreamAfterReqFinished: a. https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/stream_base.cc#L632
7. This then fails the check on if JS is allowed to be executed (setup on shared isolate from Environment B in step 3a): a. https://chromium.googlesource.com/v8/v8/+/roll/src/execution.cc

   if (!ThrowOnJavascriptExecution::IsAllowed(isolate)) {
     isolate->ThrowIllegalOperation();
   

8. Call stack begins unwinding and eventually hits: errors::TriggerUncaughtException: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/env.cc#L1097
9. This in turn attempts to execute a fatal_exception_function.As<Function>()->Call: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/node_errors.cc#L1126-L1127 a. Since JS is still disallowed we get another v8::internal::Isolate::ThrowIllegalOperation triggered
10. This unwinds and executes the TryCatchScope dtor setup in the above scope: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/node_errors.cc#L1116-L1117
11. The TryCatchScope::~TryCatchScope then builds the message to dumps and eventually causes the env_->Exit to be called: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/node_errors.cc#L572
12. This then runs the Environment::Exit and eventually will exit the process: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/env.cc#L1548