Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebView of the same vscode plugin not loading in second electron instances #13107

Open
eneufeld opened this issue Nov 27, 2023 · 13 comments
Open
Labels
electron issues related to the electron target

Comments

@eneufeld
Copy link
Contributor

eneufeld commented Nov 27, 2023

Bug Description:

Running Theia in Electron (e.g as Blueprint) twice and trying to open the same WebView in both instances fails. The first instances opens the WebView the second one fails with:

Uncaught (in promise) DOMException: Failed to register a ServiceWorker: The document is in an invalid state.

The error is thrown in:
host.js
navigator.serviceWorker.register("service-worker.js").then((async a=>{

Steps to Reproduce:

  1. Install the Draw.io Integration extension (https://open-vsx.org/extension/hediet/vscode-drawio)
  2. start a second theia blueprint instance
  3. Run the "Draw.io: New Draw.io Diagram" Command and leave the diagram view open
  4. switch to second Theia instance
  5. Run the "Draw.io: New Draw.io Diagram" Command again and see a view opening but no content rendered, see the logged error in the Developers Tools

Instead of Theia Blueprint you can also just use the Theia Electron example app to reproduce the issue.
The same does not happen in VSCode.

Additional Information

  • Operating System: Linux Ubuntu 22.04
  • Theia Version: 1.43.0 Beta
@tsmaeder
Copy link
Contributor

Just to be sure: "a second instance" is a whole new process, not just a new window, right?

@eneufeld
Copy link
Contributor Author

yes, it should be as I e.g open blueprint in a new folder calling theia ./foo

@msujew
Copy link
Member

msujew commented Nov 28, 2023

Since this only happens when starting the application multiple times, this is related to #10890.

@msujew msujew added the electron issues related to the electron target label Nov 28, 2023
@tsmaeder
Copy link
Contributor

From all I'm reading I get that running multiple process on the same data directory (containing indexdb and other files) is not supported in electron. Note that VS Code does not seem to support this either: if you do code on the command line a second time, you do not get a node code.exe process.
Of course, we could create a separate data area for each electron main process instance, but then sharing and persisting stuff per user, for example via the localStorage would not work anymore. I would be interested in knowing the use case we're trying to cover by starting multiple processes.

@msujew
Copy link
Member

msujew commented Nov 28, 2023

@tsmaeder I believe #10890 explains one of these use cases. I.e. starting two Theia instances with a different set of env variables. IMO a weird use case, but I didn't focus on the issue any further, so nothing came out of it.

@tsmaeder
Copy link
Contributor

Yeah, but you'd have to use a different app data path. Just using the same app data area in two different processes is a recipe for disaster. It's never going to work because it's not supposed to work 🤷

@tsmaeder
Copy link
Contributor

If the idea is just to let the user open more windows via theia ./myWorkspace the secondInstance even on the app has all the info we need. If we really need to run two copies, we'd have to have some support for using a different data area (cli or a profile mechanism like Firefox has), but running twice on the same data are is not going to fly, IMO.

@msujew
Copy link
Member

msujew commented Nov 28, 2023

I think this discussion is more appropriate in #10890. We should be able to fix both issues with a single change though.

@sdirix
Copy link
Member

sdirix commented Nov 28, 2023

Seems to be a long standing issue: #10693. Would be interested in "why" the registering fails.

This issue can sometimes be encountered in VS Code as well, as seen here: microsoft/vscode#125993. However, with multiple instances in Theia, we can reproduce it consistently.

@tsmaeder
Copy link
Contributor

At least in some instance in the linked issue it seems to be zombie instances of the code process hanging around. There is an issue against chromium mentioning the error message: https://bugs.chromium.org/p/chromium/issues/detail?id=1102209. However, I don't think this is the case we are hitting here.

@tsmaeder
Copy link
Contributor

@sdirix I don't think we're doing anything wrong in setting up the service workers. We can try to debug the chromium source code to understand why exactly this error occurs, but since this is not something we as a group are competent in, this might easily take a week (or more), and we would be debugging a use case that is documented as not supported. Also, there is no guarantee that we will find out what the problem is, and if we might find out that this does not work because it's not supposed to. I'm not sure going down this rabbit hole is a good investment if we're not sure we need it.
I think investigating whether using a different data area fixes the problem would be more useful.

@tsmaeder
Copy link
Contributor

tsmaeder commented Dec 4, 2023

More evidence that running more than one electron main process is a bad idea: you get race conditions from the IDB API:

023-12-04T08:58:46.601Z root ERROR Failed to restore monaco themes UnknownError: Internal error opening backing store for indexedDB.open.
log	@	logger-protocol.ts:1

Also, there is an oldish issue against electron with an explanation: electron/electron#2493

@tsmaeder
Copy link
Contributor

tsmaeder commented Dec 5, 2023

My recommendation would be to implement three new behaviors in Theia:

  • Always refuse to run when the single instance lock can’t be acquired: sharing a user data area simply cannot be made to work reliably with the underlying technology
  • Adopt the VS Code behavior where we open new windows when Theia is started a second time
  • Add a cli parameter “--userDataArea=” that allows us to give a non-default user data area for the app. This will cover the use cases where we want to run with different environment variables.

tsmaeder added a commit to tsmaeder/theia that referenced this issue Dec 5, 2023
Part of eclipse-theia#13107

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <[email protected]>
tsmaeder added a commit that referenced this issue Dec 20, 2023
…ata' path

Part of #13107

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
electron issues related to the electron target
Projects
None yet
Development

No branches or pull requests

4 participants