electron-builder icon indicating copy to clipboard operation
electron-builder copied to clipboard

Fail to install update because of app process running (Windows)

Open rossicler-hostalky opened this issue 5 months ago • 13 comments

  • Electron-Builder Version: ^23.6.0
  • Electron-Updater Version: ^5.3.0
  • Node Version: 16.x
  • Electron Version: ^23.0.0
  • Electron Type (current, beta, nightly):
  • Target: Windows (nsis)

I'm having an issue with WIndows when trying to install the update.

The problem is that after quitting the app, there's a process "hanging" that is not closed after quitting the app, which gives the user an error when trying to update the app.

I know that this issue is not on electron-updater side, but I'm wondering if there's anything I can do to force electron-updater to close any running process of my application, because this process that doesn't quit is a bug under investigation in another library, and it doesn't seem like they will fix it quickly.

Examples: When I run .quitAndInstall(), the UI will show to the user that the application is running and he needs to close, and it won't work unless the user go to task manager and close the running process.

autoUpdater.on('update-downloaded', (info) => {
  sendStatusToWindow(ChannelsTypes.UPDATE_DOWNLOADED, info.version);
  setTimeout(() => autoUpdater.quitAndInstall(), 500);
});

I also tried adding isSilent and isForceRunAfter to true, but what happens is that the app closes to install in silent mode, but it doesn't open again (probably because it failed to install), but since it's on silent mode, it won't show any errors to the user.

autoUpdater.on('update-downloaded', (info) => {
  sendStatusToWindow(ChannelsTypes.UPDATE_DOWNLOADED, info.version);
  setTimeout(() => autoUpdater.quitAndInstall(true, true), 500);
});

rossicler-hostalky avatar Feb 01 '24 18:02 rossicler-hostalky

I think this may be fixed in the next electron-updater package via https://github.com/electron-userland/electron-builder/pull/7955

mmaietta avatar Feb 02 '24 20:02 mmaietta

@mmaietta Do you have an idea when this PR will be included in a release? I checked next version (v6.1.8) and it doesn't seem to include this PR, is that correct?

rossicler-hostalky avatar Feb 05 '24 21:02 rossicler-hostalky

My apologies, I didn't realize that PR was in app-builder-lib for some reason. The PR changes are in next electron-builder, more specifically v24.11.0

mmaietta avatar Feb 05 '24 22:02 mmaietta

@mmaietta thanks the quick reply. I tried again with electron-build to v24.11.0 and electron-updater v6.1.8 and still didn't work.

FYI, I can see that the process hanging has a long name, which according to #7955 might be a problem, but since this one supposedly fixed the issue, I'm not sure if the problem is something else. To be more exact, the process hanging name has 28 characters.

Tomorrow I will try renaming my application to a smaller name and try again, and reply back here with my findings. Thanks again for the support.

rossicler-hostalky avatar Feb 06 '24 23:02 rossicler-hostalky

@mmaietta I just tested and even with a small process name (8 characters), it stills fails to kill the process and install the update. Any idea what else I can do?

EDIT: I also tried running without silent mode, but same result. image

EDIT 2: I'm digging a little bit more on this, and by checking the file allowOnlyOneInstallerInstance.nsh, I see that this script runs tasklist to get the all process from the application, and taskill to terminate them (if needed). I tried "simulating" this in a windows machine, and by running cmd /c tasklist /FI "IMAGENAME eq <my_app>", I can see the process that needs to be killed, but when I run cmd /c taskkill /f /im <my_app>, it says:

ERROR: The process with PID <number> could not be terminated.
Reason: There is no running instance of the task.

That is probably the issue that this script is encountering and why it can't kill the process.

I tested and I can successfully kill the process by running wmic process where name="<my_app>" call terminate. Would this be a valid option to replace taskkill used? If so, I'm happy to create a PR for it.

rossicler-hostalky avatar Feb 07 '24 15:02 rossicler-hostalky

I have applied the "workaround" mentioned above using patch-package and now it's all working in my end. In summary I updated the file allowOnlyOneInstallerInstance.nsh by replacing all taskkill commands to wmic process where name="<my_app>" call terminate. The only "issue" about this is that it doesn't have some filters that taskkill had, like /fi "PID ne $pid or /fi "USERNAME eq %USERNAME%".

rossicler-hostalky avatar Feb 08 '24 15:02 rossicler-hostalky

@mmaietta any input on this?

rossicler-hostalky avatar Mar 06 '24 15:03 rossicler-hostalky

Hmmm, we need the username filter to make sure we don't kill all apps by other users logged in on the system, right? This would be applicable for user-isolated installations (INSTALL_MODE_PER_ALL_USERS === null)

I'm not too familiar with nsis scripting or with wmic to make a truly informed input here though

mmaietta avatar Mar 06 '24 17:03 mmaietta

I'm not very familiar with it either, and I forgot most things I found when I did this. But as far as I remember, there are ways to filter by username, but it's not a one liner like taskkill.

Feel free to close this issue if you don't think this will be a problem for other users. If I have some time in a few days, I can try to get back at this and add the filter by username, then I could create a PR.

rossicler-hostalky avatar Mar 11 '24 18:03 rossicler-hostalky

This is not a usable solution, creates issues when auto upgrading the application as it creates a race condition between old and new instances.

Ahbrown41 avatar Mar 21 '24 02:03 Ahbrown41

This is not a usable solution, creates issues when auto upgrading the application as it creates a race condition between old and new instances.

What do you mean by "creates a race condition"? Both commands terminate the old instances before applying the update. The only issue I can see is what was discussed above.

rossicler-hostalky avatar Mar 27 '24 15:03 rossicler-hostalky

So, if I utilize the task kill command, when I have a lot of devices out in the field, it looks as though, as the device is transitioning between the old version and the new version that there are occasions when this command is killing both the old and the new versions and then no version is running. Appears to be worse on slower machines.

My guess is this has something to do with the transition between the old version and the new version.

Ahbrown41 avatar Mar 27 '24 16:03 Ahbrown41

Ahbrown41

I see, so it still might be a problem with the filters, since it's killing based on the name, and not PID, it might kill both if they are running at the same time. If my assumption is correct, it's still all tied to the filters missing, which is possible to implement, but in my solution, I didn't.

rossicler-hostalky avatar Apr 29 '24 19:04 rossicler-hostalky