-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
NSIS installer always show "XYZ cannot be closed. Please close it ..." when there is other process name contains "XYZ" #6865
Comments
This problem does not exist on version 22.14.13 or previous. |
23.1.0 version , also not working |
same here, version with 23.0.3 |
Getting the same error when trying to change the install location to Originally using 23.0.3, but tried upgrading to 23.3.1 & 23.3.2 (pre-release), but both also had this issue. Interestingly if right clicking on the .exe installer and selecting Also tried downgrading to 22.14.13, but got a different error: Using the custom path described in docs: https://www.electron.build/configuration/nsis.html#common-questions EDIT: main goal to install to Program Files was resolved by simply setting |
Saw this in our project too - still a problem in v23.3.3. From a bit of looking around and experimenting it seems that the addition of With that line removed the produced installer works fine. |
* Downgrading the electron-builder to avoid electron-userland/electron-builder#6865 * Removed "git clean" workaround from CI.
This PR reverts two version bumps from #3712: * `yargs` needs to be downgraded, because a never version can trigger evanw/esbuild#2441 * `electron-builder` needs to be downgraded, because our workaround for electron-userland/electron-builder#6865 proved insufficient. As the upstream issues are resolved, we should bump these dependencies again.
Hi. Any solutiuon exist or need to downgrade? |
I'm fairly impressed that this issue isn't high priority right now... Currently downgrading as this is a breaking bug for our Windows users. A solid 5% of our users got hit with this bug which can't happen in a production build ofcourse |
For the developer picking this up, somehow FIND_PROCESS inside EDIT 2: Alright so the issue is the following: the exit code of FIND_PROCESS is 0. This is probably because there is a parent process active which doesn't fall under the same name. It's on admin level so the user can't close it as it doesn't know the name, we can't close it as it's on admin level and it keeps looping. I think the solution to this problem is to show a list of running processes and display the list of processes blocking the installer instead of a simple retry.
|
Anyone have a solution ? |
Definitely not a solution, but you can include a custom script with a macro so app-builder-lib uses this macro instead of the default one. In add a path to a custom .nsh script file in your package.json under build: Inside the installer.nsh just add:
This will bypass the WHOLE check. You can add the code from allowOnlyOneInstallerInstance.nsh and edit it so it works for you. I don't have enough knowledge to propose permanent a solution but this is a step for people to make their own and create a PR to fix it! To iterate it again: THIS WILL BYPASS THE WHOLE RUNNING CHECK SO THIS MIGHT CORRUPT FILES IF THE PROGRAM IS ALREADY OPEN! |
Quick note. This was added to resolve these (based off the commit message): I'm wondering if there's a way for us to identify it other than "contains" and instead look for an exact match? Not sure if that's where the bug is originating though. At a base level, my apologies for this issue being stagnant. I've been the only maintainer on this project for some time now and am largely dependent upon community contributions, especially when relating to windows and linux (I'm predominantly a mac user). |
The weird thing is, even on brand new systems with nothing on it this bug will happen. I have verified the results and every user who reported this error has a clear prompt of the exact tasklist command while it still returns 0 inside the nsis installer. At this point i'm guessing it's find.exe or the ns exec plugin for nsis doing something weird rather than the tasklist command. The tasklist command always returns 0 as result so we can't exclude find.exe without a replacement. I'll take a look myself next week to research all of the nsis file changes but it's really hard to test as I haven't been able to replicate it. (I'm already on my 3rd VPS and still no luck) |
Soooo circling back, what do we do here? Completely rewrite the "Application still running" logic? Or just revert the implementation that was introduced in electron-builder 23.x.x? |
Curious. Would anyone mind trying out this?
With:
I think that'll provide the return value info we're looking for? Curious if this would fix the issue, but I have no manner with which to test |
In my case, If I set @mmaietta Is there any solution to prevent this error? |
I'm going to try explaining what I think the relevant portions of the NSIS script are doing and what a probable issue is. (I'm not certain about this). My understanding of the relevant parts of the NSIS scriptAn explanation of relevant parts of the NSIS scriptThe
|
!insertmacro FIND_PROCESS "${APP_EXECUTABLE_FILENAME}" $R0 | |
${if} $R0 == 0 | |
${if} ${isUpdated} | |
# allow app to exit without explicit kill | |
Sleep 1000 | |
Goto doStopProcess | |
${endIf} | |
MessageBox MB_OKCANCEL|MB_ICONEXCLAMATION "$(appRunning)" /SD IDOK IDOK doStopProcess | |
Quit |
Thus, if | find.exe "${_FILE}"
exits with status 0, we assume that the app executable is still running.
A possible issue
Notice that running app processes are killed like this:
doStopProcess:
DetailPrint `Closing running "${PRODUCT_NAME}"...`
# https://github.com/electron-userland/electron-builder/issues/2516#issuecomment-372009092
!ifdef INSTALL_MODE_PER_ALL_USERS
nsExec::Exec `taskkill /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid"`
!else
# ↓ This line! ↓
nsExec::Exec `cmd /c taskkill /im "${APP_EXECUTABLE_FILENAME}" /fi "PID ne $pid" /fi "USERNAME eq %USERNAME%"`
# ↑
# Notice the "PID ne $pid"
!endif
where /fi "PID ne $pid"
causes the command to kill only processes that don't have the same PID as this one (where $pid
is defined at the beginning of _CHECK_APP_RUNNING
to be the pid
of the current process).
Thus, we should (probably) also be filtering the current process' PID out of the list in FIND_PROCESS
.
A patch
I'm currently using this patch:
diff --git a/templates/nsis/include/allowOnlyOneInstallerInstance.nsh b/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
index a1fd1875d852ff69c087a3103eff827c20d37ca5..5222614ddad3276876857e7a9dde4017a6b9fc85 100644
--- a/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
+++ b/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
@@ -42,7 +42,7 @@
${nsProcess::FindProcess} "${_FILE}" ${_ERR}
!else
# find process owned by current user
- nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
+ nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "PID ne $pid" /FI "IMAGENAME eq ${_FILE}" | %SYSTEMROOT%\System32\find.exe "${_FILE}"`
Pop ${_ERR}
!endif
!macroend
It seems to work, but it's based on the assumption that, without this change, the tasklist includes the installer, which might not be true.
Edit: Given this comment:
I'm wondering if there's a way for us to identify it other than "contains" and instead look for an exact match? Not sure if that's where the bug is originating though.
The weird thing is, even on brand new systems with nothing on it this bug will happen. I have verified the results and every user who reported this error has a clear prompt of the exact tasklist command while it still returns 0 inside the nsis installer. At this point i'm guessing it's find.exe or the ns exec plugin for nsis doing something weird rather than the tasklist command. The tasklist command always returns 0 as result so we can't exclude find.exe without a replacement. I'll take a look myself next week to research all of the nsis file changes but it's really hard to test as I haven't been able to replicate it. (I'm already on my 3rd VPS and still no luck)
My guess is that the above patch does not work.
Footnotes
We've (https://www.codux.com/) been hit quite hard with this one. Our investigation found several possible changes that caused this: We've forcibly ran the uninstaller in non-silent mode from installer (e.g. removed the /S) and saw it failing to remove the application directory. Tried several workarounds. We're currently trying to define EDIT: we're adding the following .nsh script using !macro customRemoveFiles
DetailPrint "Removing files..."
RMDir /r $INSTDIR
!macroend |
Alright, more info. Our installation contained a file with a really long file path (247 characters long). This caused the rename operation to fail on some machines (depending on the user name, where the temp dir is, etc). It seems that when the uninstaller fails, the installer assumes it's because the app is still running, giving a wrong error message? Not sure about the above statement ^, but it wouldn't surprise me if that's the culprit of many failures that were reported here. |
It is possible to get same error during first clean installation if install path + file path in the installation is greater than 256. If an installation package contains really long file paths then install path can be the last drop to push file path length over 256 limit when copying files to install location. |
Yes, the error message is incorrect. In our case it was too long file paths. |
Any workarounds exists? Now i downgrade to 22.14.13 and it works. |
The 'customRemoveFiles' solution as suggested by @AviVahl did the trick for us, but I think this is not much different from downgrade to version 22.14. Using customRemoveFiles completely bypass the atomic renaming stuff (and I think it is a good thing to have... when it works). |
Same issue. |
This PR avoids #8119 by selectively bumping the `electron-builder` on macOS CI runners. We do this only on macOS, as we do not want to trigger electron-userland/electron-builder#6865 on Windows.
StrCpy and RM and other cmd not support long file path, !macro customInit
Delete "$INSTDIR\Uninstall*.exe"
!macroend |
I'm having this issue also, tried most of the fixes suggested but the only thing that worked was reverting to "electron-builder": "22.14.13" |
@mirkin The same. Only reverted to this version works as excepted. |
I think this chunk of code was changed in this commit. I don't have knowledge on nsh script. maybe someone can check this. |
Weird, renaming the file to a really short name, make it work. |
I'm also having this problem, we have had no updates yet? |
I need help recreating this issue via a minimum reproducible repo, then I can debug into this |
This is still an issue |
tested this solution (workaround) on windows 11 electron builder 25.0.4 and it is working, the cause of the issue was long user name on windows account |
Also having these issues with 25.0.5, seriously breaking. |
Will report back on how this goes |
I had the same problem. The installer would work as expected on the first install, but on the 2nd install would produce this error.
This workaround worked for me. Tested with "electron": "^32.2.0", "electron-builder": "^25.1.7". On Windows 8, 10, 11 |
In my case, productName of my project is
XYZ
, it allows user to install a service namedXYZ Helper
on Windows, which runs when the operating system starts up. When I use electron-builder(23.0.3) to package(target: NSIS), the installer always showXYZ cannot be closed. Please close it manually and click retry to continue.
even after I'm sure to quit XYZ completely. Only after I also stop the service namedXYZ Helper
, this step can be processed. But XYZ Helper is not part of XYZ, which is a separated process and should not be stopped. electron-builder(22.14.13) doesn't have this problem.The text was updated successfully, but these errors were encountered: