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

[single-instance] doesn't shutdown immediately on Windows #1017

Closed
Legend-Master opened this issue Mar 4, 2024 · 7 comments · Fixed by #1766
Closed

[single-instance] doesn't shutdown immediately on Windows #1017

Legend-Master opened this issue Mar 4, 2024 · 7 comments · Fixed by #1766
Labels
documentation Improvements or additions to documentation platform: windows Windows specific issues plugin: single-instance

Comments

@Legend-Master
Copy link
Contributor

Legend-Master commented Mar 4, 2024

When using single instance plugin on Windows, the second instance doesn't shutdown immediately

Windows implementation uses app.exit while the Linux one uses std::process::exit, app.exit only sends a shutdown signal, and the app will still be running through all the following setup functions, e.g. create the window, create the system tray...

This was changed to this in 4d903f3 for cleaning up system tray, but currently it doesn't even trigger exit event for some reason (even if it actually triggers it, it's still problematic to have flashing windows)

In my opinion, we should just use std::process::exit here (maybe run with cleanup_before_exit to clean up system tray) (simple fix but may not be perfect) or we'll need a way to shutdown the app cleanly in setup step from Tauri

@FabianLars
Copy link
Member

In my opinion, we should just use std::process::exit here (maybe run with cleanup_before_exit to clean up system tray)

If we're talking about v1 this is exactly the same thing as app.exit(). https://docs.rs/tauri/1.6.1/src/tauri/app.rs.html#524-527
In v2 this will trigger the Exit and ExitRequested RunEvents so we may indeed want to split it up into cleanup && process::exit to skip those events.

Anyway, i'm fairly sure that this has nothing to do with how the app is exited, but when. The .setup hook just feels too late for this though it's a bit weird that it's more of an issue on windows than linux 🤔

@FabianLars FabianLars added the bug Something isn't working label Mar 4, 2024
@FabianLars FabianLars changed the title Single instance doesn't shutdown immediately on Windows [single-instance] doesn't shutdown immediately on Windows Mar 4, 2024
@Legend-Master
Copy link
Contributor Author

If we're talking about v1 this is exactly the same thing as app.exit().

Oh, I wasn't aware of that, I just started to build a Tauri app a month ago with Tauri 2 beta

The .setup hook just feels too late for this though

Yep, but that's the first chance a plugin can access app, if we want to do it earlier, we'll probably need an extra API from Tauri side

it's a bit weird that it's more of an issue on windows than linux

The implementation for Linux is std::process::exit(0) while the Windows one is app.exit(0), I don't think the problem has anything to do with the platform

Maybe changing both of them to this for now?

app.cleanup_before_exit()
std::process::exit(0)

@FabianLars
Copy link
Member

Yep, but that's the first chance a plugin can access app, if we want to do it earlier, we'll probably need an extra API from Tauri side

This is probably the best approach, otherwise we cannot guarantee the time when this plugin runs. With how heavy v2 is on plugin use this could mean that 10 other plugins already ran their setup hook.
cc @lucasfernog

@FabianLars
Copy link
Member

FYI, We currently load setup functions in their added order, so as long as we register single instance plugin first, its setup function is guaranteed to run first

Which is not something we can rely on at all so it's still something we should think about.

Anyway, the new Exit/ExitRequested sensitive app.exit() behavior indeed slows it down quite a bunch, way more than expected, so if you'd like to work on it we'd appreciate a PR (changing it to cleanup + exit as you showed above).

@FabianLars
Copy link
Member

I'll keep this issue open until Lucas had a chance to react to the order thingy. Worst case we just add documentation for this and wait and see if more plugins/users need some kind of priority thing.

@FabianLars FabianLars added documentation Improvements or additions to documentation and removed bug Something isn't working labels Aug 7, 2024
@FabianLars
Copy link
Member

Since Lucas didn't react i marked this as an documentation issues now meaning that we'll document that it should be one of the first plugins to register.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation platform: windows Windows specific issues plugin: single-instance
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants