Closing New Window While Sync is Running Freezes app

sliceofbrad · July 8, 2024, 7:15pm

Steps to reproduce

Activate Obsidian Sync (if it’s not already)
Right click on a link and open it in a new window
Make some changes to the file opened in a new window.
Manually save.
While Sync is still running (as indicated by the sync icon), close the new window.

Did you follow the troubleshooting guide?

Yes. On a completely clean vault with nothing installed and Sync Set up, I can replicate this freeze. Similarly, if I turn sync off, I can stop this.

Expected result

New window closes while sync is running, sync completes, app doesn’t freeze.

Actual result

App freezes and hangs for several minutes. Only indication of any progress is the slow spinning of the sync icon. Sometimes sync will eventually catch up, but the total time seems to depend on vault size. In a fresh sandbox vault this takes around 1-2 minutes, on a vault with a few thousand notes it takes closer to 10-20.

If I wait a few seconds for sync to complete before I close the new window, there’s no freeze. Similarly, if sync is disabled, I can’t reproduce this.

Environment

SYSTEM INFO:
Obsidian version: v1.6.5
Installer version: v1.6.5
Operating system: Windows 10 Enterprise 10.0.19045
Login status: logged in
Catalyst license: none
Insider build toggle: off
Live preview: on
Base theme: adapt to system
Community theme: none
Snippets enabled: 0
Restricted mode: on

RECOMMENDATIONS:
none

Additional information

WhiteNoise · July 9, 2024, 8:12pm

thanks.

Upstream reports:

github.com/electron/electron

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

opened 08:50PM - 10 Jan 23 UTC

markh-discord

platform/windows crash

bug

has-repro-gist 13-x-y 17-x-y 22-x-y 24-x-y

### Preflight Checklist - [X] I have read the [Contributing Guidelines](https…://github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project. - [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to. - [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) 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 1. 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 2. 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 3. 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 4. 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. 5. 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 6. 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(); ``` 7. Call stack begins unwinding and eventually hits: `errors::TriggerUncaughtException`: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/env.cc#L1097 8. 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 9. 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 10. 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 11. This then runs the `Environment::Exit` and eventually will exit the process: https://github.com/nodejs/node/blob/2af83ea0f8a1594c0d0bf4bc4b3035234bcd895e/src/env.cc#L1548

github.com/electron/electron

[Bug]: Closing Node file watcher just before removing iframe throws illegal access errors

opened 03:45PM - 23 Nov 23 UTC

dmx-patrick

bug

component/node-integration platform/all status/confirmed has-repro-gist 26-x-y 27-x-y 28-x-y

### Preflight Checklist - [X] I have read the [Contributing Guidelines](https:/…/github.com/electron/electron/blob/main/CONTRIBUTING.md) for this project. - [X] I agree to follow the [Code of Conduct](https://github.com/electron/electron/blob/main/CODE_OF_CONDUCT.md) that this project adheres to. - [X] I have searched the [issue tracker](https://www.github.com/electron/electron/issues) for a bug report that matches the one I want to file, without success. ### Electron Version 27.1.2 ### What operating system are you using? Windows ### Operating System Version Windows 11 Pro 22H2 ### What arch are you using? x64 ### Last Known Working Electron version _No response_ ### Expected Behavior Removing an iframe after closing a file watcher should not throw any errors. ### Actual Behavior When you close a file watcher just before removing an iframe you see 2 or 3 Uncaught illegal access errors in the console. A workaround is to do the iframe removal first and then close the file watcher. ### Testcase Gist URL https://gist.github.com/dmx-patrick/a176609a8664c2e98599b710545c8997 ### Additional Information _No response_

WhiteNoise · July 10, 2024, 4:05pm

A workaround for this will be introduced in v1.6.7

system · August 7, 2024, 4:06pm

This topic was automatically closed 28 days after the last reply. New replies are no longer allowed.