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

Use Electron's single instance lock by default #10890

Closed
msujew opened this issue Mar 17, 2022 · 8 comments · Fixed by #13831
Closed

Use Electron's single instance lock by default #10890

msujew opened this issue Mar 17, 2022 · 8 comments · Fixed by #13831
Assignees
Labels
electron issues related to the electron target
Milestone

Comments

@msujew
Copy link
Member

msujew commented Mar 17, 2022

Originally inspired by this thread

By default, the electron backend does not use the singleInstance lock. Consequently, clicking on the app shortcut starts another Theia process, which doesn't have access to the default index.db file, since each backend app locks their instance of the index.db. It will therefore create its own, with all application wide settings reseted (layout, locale, etc.).

I propose to set singleInstance to true by default and refactor the onSecondInstance method to simply start a new workspace with the supplied arguments.

@msujew msujew added the electron issues related to the electron target label Mar 17, 2022
@msujew
Copy link
Member Author

msujew commented Mar 17, 2022

cc @colin-grant-work since you were dealing with a lot of application lifecycle changes recently. What do you think of that idea? I think opening a new window on the existing process is a more intuitive solution. That's also the way VSCode is handling it.

@colin-grant-work
Copy link
Contributor

@gbodeen, you've dealt more with the impact of our current multi-instance setup on usability - what are your thoughts here? @kenneth-marut-work and I also observed that at least one bug related to multiple instances seemed to disappear after the recent Electron uplift along with the theme flash and warning messages associated with failure to access the indexed database instance, so it may be that Electron has made some improvements in this area, as well. Whatever the case, I agree that maintaining a single instance is likely to be preferred.

@gbodeen
Copy link
Contributor

gbodeen commented Mar 19, 2022

@gbodeen, you've dealt more with the impact of our current multi-instance setup on usability - what are your thoughts here?

The reason we abandoned the singleInstance lock was that some users wanted to have multiple running Theia apps with access to different environment variables. My understanding is that's not possible with a single shared backend process, at least not without additional work in Theia to distinguish between environments. Relatedly, in singleInstance mode, we found that many messages from the backend were sent to all the frontends instead of distinguishing which one they should go to.

@kittaakos
Copy link
Contributor

Related #7983

@sdirix
Copy link
Contributor

sdirix commented Jan 30, 2024

We discussed this today in the developer meeting. We concluded that the current default of secondary Electron instances is not optimal as users will immediately run into problems like #13107 and #7983.

The proposal is to switch to a single process approach by default: So opening another Electron "instance" (e.g. by double clicking a bundled Electron application, or opening another instance via command line) will instead open another window in the first instance. This is the same approach which VS Code uses.

Should applications need "real" multi instances, they can easily overwrite this and fallback to the old approach or use the new user data option which was recently merged (#13155) for a cleaner multi instance support which also avoids the mentioned problems.

@sdirix
Copy link
Contributor

sdirix commented Jan 30, 2024

@msujew, you mentioned that you might be able to contribute this. If you run out of time, let me know, then I could also take it over 👍

@msujew msujew self-assigned this Jan 30, 2024
@rschnekenbu
Copy link
Contributor

@msujew, this issue came to us again. Do you think you would have time to work on it or should we take it over?

@msujew msujew removed their assignment Jun 11, 2024
@msujew
Copy link
Member Author

msujew commented Jun 11, 2024

@rschnekenbu I've unassigned myself.

tsmaeder added a commit that referenced this issue Jun 26, 2024
Fixes #10890

Also implements VS Code-like behavior when opening second instances

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@jfaltermeier jfaltermeier added this to the 1.51.0 milestone Jun 27, 2024
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

Successfully merging a pull request may close this issue.

8 participants