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

Updater leaves app in unrecoverable state if update is performed during sign out or shut down #7294

Closed
davej opened this issue Nov 30, 2022 · 19 comments
Labels

Comments

@davej
Copy link
Contributor

davej commented Nov 30, 2022

  • Electron-Updater Version: 5.3.0
  • Target: Windows NSIS

If the NSIS Windows updater is running during a shutdown or log-off, the application directory will likely be left empty (or missing some files). This leads to the app being unusable.

It is common for users to quit an app (and trigger an update) before shutting down or logging off the computer so this seems like this could be quite a common issue. We had multiple reports of this from customers and I was able to reproduce it myself also.

Presumably, the way to fix this would be to have the app installer not respond to requests to exit until install has been completed. This would cause the "Not responding" prompt to show and the user would then need to explicitly "End Task". I'm willing to jump into this and try and fix it with a bit of guidance. Is it possible to fix this in electron-updater or would this need to be fixed upstream in NSIS?

This is a duplicate of #3798 but that issue has been closed as stale.

@mmaietta
Copy link
Collaborator

mmaietta commented Dec 1, 2022

I was under the impression that this would appear if the user tries to shutdown/log-off during the app running
#7251

During an update, specifically, we spawn a thread that runs the installer. After that, electron quits and AFAIK, there's no way for a spawned thread to block shutdown. The spawned thread needs to be detached otherwise electron can't quit/kill the thread on quit AFAIK
https://github.com/electron-userland/electron-builder/blob/master/packages/electron-updater/src/NsisUpdater.ts#L138

@jeremyspiegel
Copy link
Contributor

jeremyspiegel commented Dec 16, 2022

The change in #7251 only applies if the upgrade is not running silently (though I have verified that it works in that case). But we still see the problem when the upgrade runs silently on app quit.

@davej
Copy link
Contributor Author

davej commented Dec 16, 2022

@jeremyspiegel @mmaietta Any reason why we couldn't take it out of the ${IfNot} ${Silent} conditional block?

@jeremyspiegel
Copy link
Contributor

I just tried it and it didn't work for me. I also tried creating a new window as suggested in this stackoverflow post but that also didn't work. I'll try playing around with it a bit more to see if I can get something working.

@jeremyspiegel
Copy link
Contributor

jeremyspiegel commented Dec 20, 2022

When using that code and adding a sleep loop afterward, I am able to see the shutdown UI is blocked with the reason string. However, if the installer is allowed to continue, Windows will continue with the shutdown and leave the app in an uninstalled or partially installed state.

I haven't verified yet but I think this is because in addition to calling ShutdownBlockReasonCreate, we also need to return FALSE in response to WM_QUERYENDSESSION. The default window handler for the window I created in order to call ShutdownBlockReasonCreate will return TRUE in response to WM_QUERYENDSESSION (as soon as window messages are dispatched, which presumably nsis does even in a silent install).

So I think we'll need an nsis plugin to implement this.

@davej
Copy link
Contributor Author

davej commented Dec 20, 2022

@jeremyspiegel Great debugging. Just to clarify, this is an issue regardless of whether the installer runs silently or not?

@jeremyspiegel
Copy link
Contributor

jeremyspiegel commented Dec 20, 2022

The fix in #7251 works if the upgrade is not running silently (since nsis already returns FALSE in response to WM_QUERYENDSESSION for the window it creates).

@davej
Copy link
Contributor Author

davej commented Dec 20, 2022

@jeremyspiegel Fantastic. So appUpdater.quitAndInstall(false) should give us a workaround in the interim. I think that's enough for our internal use case, a silent update would be nice but not essential.

@jeremyspiegel
Copy link
Contributor

jeremyspiegel commented Dec 20, 2022

@davej But the default for appUpdater.quitAndInstall() is already false. Do you mean you'll set autoInstallOnAppQuit to false also, then on app quit call appUpdater.quitAndInstall()?

@jeremyspiegel
Copy link
Contributor

I created an nsis plugin that fixes the issue so that silent installs can block system shutdown while in progress: https://github.com/jeremyspiegel/shutdown-block-test

In my testing I also found that the uninstaller can fail when the system has started restarting, which would now hang the restart since a MessageBox would be shown. I didn't figure out why the uninstaller fails in this situation, but I found that the MessageBox added here should probably use the /SD flag to not show when in silent mode. I added a workaround for that by overriding customUnInstallCheck/customUnInstallCheckCurrentUser in my installer.nsh.

@github-actions
Copy link
Contributor

github-actions bot commented May 6, 2023

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label May 6, 2023
@github-actions
Copy link
Contributor

This issue was closed because it has been stalled for 5 days with no activity.

@popod
Copy link
Contributor

popod commented Oct 3, 2023

@davej Have you found a workaround for this issue ? How have you resolve it ?

@jeremyspiegel are you always using your nsis plugin ? Does it works ? and can you explains a bit more how to use it with electron-builder / electron-updater ?

Thank you in advance :)

@davej
Copy link
Contributor Author

davej commented Oct 3, 2023

@davej Have you found a workaround for this issue ? How have you resolve it ?

@popod Yes, we now use an nsis plugin to achieve this by default on our Electron CLI.

@popod
Copy link
Contributor

popod commented Oct 4, 2023

@davej Thanks for reply.

Is your plugin available/open source or is it the plugin named earlier in this issue (https://github.com/jeremyspiegel/shutdown-block-test) ?

@jeremyspiegel
Copy link
Contributor

@jeremyspiegel are you always using your nsis plugin ? Does it works ? and can you explains a bit more how to use it with electron-builder / electron-updater ?

Yes the nsis plugin worked well for me. To use it, you could just drop the build directory from https://github.com/jeremyspiegel/shutdown-block-test into your project directory and electron-builder should pick up the plugin and the dll it relies on.

@popod
Copy link
Contributor

popod commented Nov 22, 2023

@jeremyspiegel (sorry for the delay of my reply) Thank you for your message ! This helps a lot :)

@mmaietta will it not be a good option to provide this (the jeremyspiegel plugin) by default with electron-builder when auto-update is activated (Windows build) ? For me and my users, this was a big issue until now..

@mmaietta
Copy link
Collaborator

If you're willing to integrate it in and open a PR, then yes, happy to have it included. I see that only an x64 version exists in that repo, whereas we'll probably also need an ia32 and arm64 variant as well.

@popod
Copy link
Contributor

popod commented Nov 22, 2023

Thank you for your reply @mmaietta !

Unfortunately, I don't think I have the skills to implement this in electron-updater or to propose an ia32 or arm64 version of the plugin..

But I'm happy to test it on a PR, for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants