athenakia commented 2 years ago

Electron-Builder Version: 23.0.3

Node Version: 16.3.1

Electron Version: 16.2.5
Electron Type (current, beta, nightly):current

Target: Windows 11

In my case, productName of my project is XYZ, it allows user to install a service named XYZ 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 show XYZ 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 named XYZ 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.

athenakia commented 2 years ago

This problem does not exist on version 22.14.13 or previous.

qingniao927 commented 2 years ago

23.1.0 version , also not working

chemistryx commented 2 years ago

same here, version with 23.0.3

nickfujita commented 2 years ago

Getting the same error when trying to change the install location to Program Files. When using the default install location, or install to any directory outside of Program Files, am not getting this error (eg. C:\MyApp)

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 Run as administrator this issue does not come up. So not sure if this is a signing issue? I do not currently have any signing implemented for windows, but will try this.

Also tried downgrading to 22.14.13, but got a different error: Error opening file for writing: C:\Program Files\MyApp\Uninstall MyApp.exe

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 perMachine to true rather than setting manually with custom .nsh script. This issue had the same error message, but ultimately a different issue.

jdunkerley commented 2 years ago

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 args.push("-INPUTCHARSET", "UTF8") to packages/app-builder-lib/src/targets/nsis/NsisTarget.ts is causing the issue (but no idea why).

With that line removed the produced installer works fine.

sergeushenecz commented 2 years ago

Hi. Any solutiuon exist or need to downgrade?

x6pnda commented 1 year ago

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

x6pnda commented 1 year ago

For the developer picking this up, somehow FIND_PROCESS inside packages/app-builder-lib/temapltes/nsis/include/allowOnlyOneInstallerInstance.nsh says that a process is running while if you execute the same command in the command line, it reports nothing is running. It gets stuck because it reports there is a process active but it can't kill any of the processes as there are non active which causes you to be stuck in an infinite loop.

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.

Edit: Seems I have found it. When I have the installer "test-app-installer.exe" open for "Test App.exe" for an one click installation for an user installation, the allowOnlyOneInstallerInstance.nsh calls FIND_PROCESS which execs the following command: cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq Test App.exe" | %SYSTEMROOT%\System32\find.exe "Test App.exe". While having the installer open, I check this command inside my cmd and it returns 0 as exit code (so blocking installation) but if I look at the result, there are no processes active (clear result)

sandeep1995 commented 1 year ago

Anyone have a solution ?

x6pnda commented 1 year ago

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: "nsis": { "include": "installer/installer.nsh" }

Inside the installer.nsh just add:

!macro customCheckAppRunning

!macroend

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!

mmaietta commented 1 year ago

From a bit of looking around and experimenting it seems that the addition of args.push("-INPUTCHARSET", "UTF8") to packages/app-builder-lib/src/targets/nsis/NsisTarget.ts is causing the issue (but no idea why). With that line removed the produced installer works fine.

Quick note. This was added to resolve these (based off the commit message):

4898 If the path contains Chinese characters, packaging will fail.

6232 shortcutName in chinese

6259 Random code After packing

NSIS installer always show "XYZ cannot be closed. Please close it ..." when there is other process name contains "XYZ"

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).

x6pnda commented 1 year ago

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)

mmaietta commented 1 year ago

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? I'm really out of my element here so I personally am unable to do the rewrite approach. I'd love to unblock you guys such that you can update electron-builder to latest.

mmaietta commented 1 year ago

Curious. Would anyone mind trying out this? Replace the final portion of:

nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | %SYSTEMROOT%\System32\find.exe "${_FILE}"`

With:

| findstr /B /C:"INFO: " > nul

I think that'll provide the return value info we're looking for? Ref: https://stackoverflow.com/questions/56569414/process-names-over-21-characters-not-detected-supported-by-tasklist/56577365#56577365

Curious if this would fix the issue, but I have no manner with which to test

SergiiMarkin commented 1 year ago

In my case, If I set perMachine: false, then It will throw the same error when installing .exe file. All of my version is same as @athenakia .

@mmaietta Is there any solution to prevent this error?

mmaietta commented 1 year ago

@SergiiMarkin, thanks for chiming in... from re-reading this ticket again, you're saying the exact same thing as everyone else.

Two solutions:

use perMachine: true per https://github.com/electron-userland/electron-builder/issues/6865#issuecomment-1193773494
Test out this change within a custom nsis script below and report back

Best way for someone to help me here is: Create a minimum reproducible repo for me to test locally with. I have a windows machine now, but it's not a dev environment

I've created this patch via patch-package as a potential test app-builder-lib+23.0.3.patch

diff --git a/node_modules/app-builder-lib/templates/nsis/include/allowOnlyOneInstallerInstance.nsh b/node_modules/app-builder-lib/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
index cc77993..41681dc 100644
--- a/node_modules/app-builder-lib/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
+++ b/node_modules/app-builder-lib/templates/nsis/include/allowOnlyOneInstallerInstance.nsh
@@ -40,8 +40,9 @@
     ${nsProcess::FindProcess} "${_FILE}" ${_ERR}
   !else
     # find process owned by current user
-    nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | find "${_FILE}"`
+    nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" -fo csv | convertfrom-csv`
     Pop ${_ERR}
+    Pop $1.PID
   !endif
 !macroend

Honestly, I have no idea if Pop $1.PID is the correct syntax. I've tested locally just convertfrom-csv and it works locally in powershell for IMAGENAMEs that are longer than tasklist can output. So if anyone has knowledge on how to handle nsExec in macros, I'd really appreciate some guidance.

If no guidance on nsh scripting and no minimum reproducible repro are provided, then please don't expect any progress to be made from my side.

SergiiMarkin commented 1 year ago

Hi @mmaietta thanks for your reply.

As for the first solution, I am not able to use the first solution with perMachine: true. It has a problem with /allusers suffix in uninstallstring as you can see here #7472 So I had to use perMachine: false.
As for the second solution, I would like to edit my npm package app-builder-lib with your custom code from patch version. Before I use your code, I have questions. nsExec::Execcmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" -fo csv | convertfrom-csv``

Why do I have to replace | find "${_FILE} with -fo csv | convertfrom-csv?

mmaietta commented 1 year ago

convertfrom-csv returns an object that can have keys directly accessed, whereas AFAIK, find is a simple string search and doesn't capture long app names. Just curious if it'd also resolve this issue tbh

dj-nuo commented 1 year ago

Hi! Any ideas how to resolve the issue? None of the proposed approaches currently solve it for my Windows users. And I can't downgrade the version because of necessary features for "universal" Mac build((

... meaning that every time user sees an "update available" in my app - when installing on Windows this error appears - "App cannot be closed. Please close it manually and click Retry to continue"

I'm using nsisWeb installer

sergeushenecz commented 1 year ago

Hi! Any ideas how to resolve the issue? None of the proposed approaches currently solve it for my Windows users. And I can't downgrade the version because of necessary features for "universal" Mac build((

... meaning that every time user sees an "update available" in my app - when installing on Windows this error appears - "App cannot be closed. Please close it manually and click Retry to continue"

I'm using nsisWeb installer

I have some problem for users on 10 pro windows 21H1 version

dj-nuo commented 1 year ago

Ok, as a temporary solution I did this in my package.json:

...
"scripts": {
    "build:electron:mac": "npm i electron-builder@^24.4.0 && electron-builder -m --config build-mac.config.json --publish always",
    "build:electron:win": "npm i electron-builder@22.14.13 && electron-builder -w --config build-win.config.json --publish always",
...
}
...

...meaning that every time I initiate the build for Windows - I also rollback to the working electron-builder version (22.14.13)

qhkm commented 1 year ago

npm i electron-builder@22.14.13

Thanks! This is good enough workaround for now.

personalizedrefrigerator commented 1 year ago

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 script

# An explanation of relevant parts of the NSIS script ## The `cmd /c tasklist /FI ...` line If I open up `cmd.exe` and run ``` cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq cmd.exe" ``` I get something similar to this: ``` Image Name PID Session Name ====================== ======== ============= cmd.exe 1234 Console cmd.exe 5678 Console cmd.exe 7654 Console ``` The first filter, `/FI "USERNAME eq %USERNAME%"` should return only processes running under the current user (the user with `%USERNAME%`).[^1] The second filter, `/FI "IMAGENAME eq cmd.exe"` should return only processes with `cmd.exe` in the "Image Name" column. Thus, if we then run ``` cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq cmd.exe" | find "cmd.exe" ``` only lines that contain "cmd.exe" should be printed out: ``` cmd.exe 1234 Console cmd.exe 5678 Console cmd.exe 7654 Console ``` Similarly, [as can be seen by printing `%errorlevel%`](https://stackoverflow.com/a/334890), `find` has exit status `0` when matches are found and non-zero exit status when no matches are found.[^2] [^1]: `tasklist` documentation: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/tasklist#parameters [^2]: `find` documentation: https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/find#exit-codes ## What the `nsExec::Exec ... Pop ${_ERR}` part is (probably) doing According to the [NSIS documentation](https://nsis.sourceforge.io/Docs/Chapter4.html#plugindlls)[^3], NSIS maintains a stack where ``` Push "something here" ``` pushes values onto the stack and ``` Pop ${foo} ``` pops values off the top of the stack and stores them in the variable `foo`. Presumably, ``` nsExec::Exec `cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | find.exe "${_FILE}"` Pop ${_ERR} ``` executes the command ``` cmd /c tasklist /FI "USERNAME eq %USERNAME%" /FI "IMAGENAME eq ${_FILE}" | find.exe "${_FILE}" ``` and `nsExec::Exec` stores its exit status on the stack. Hence, `Pop ${_ERR}` moves the exit status from the stack into the variable `_ERR`. Observe that `FIND_PROCESS` is being used like this: https://github.com/electron-userland/electron-builder/blob/84ed3ff123b5ae92cd3350d64677434f9b397b76/packages/app-builder-lib/templates/nsis/include/allowOnlyOneInstallerInstance.nsh#L58-L66 Thus, if `| find.exe "${_FILE}"` exits with status 0, we assume that the app executable is still running. [^3]: https://nsis.sourceforge.io/Docs/Chapter4.html#plugindlls

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.

AviVahl commented 1 year ago

We've (https://www.codux.com/) been hit quite hard with this one. Several of our users have reported failures to install our new version over the old one with the same error dialog. The uninstaller, which gets launched by the installer, fails 5 times. Launching the uninstaller manually from "Apps/Installed-Apps/Add-Remove" works.

Our investigation found several possible changes that caused this: https://github.com/electron-userland/electron-builder/pull/6551 https://github.com/electron-userland/electron-builder/pull/5902

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 customRemoveFiles to RMDir /r $INSTDIR and see if it helps.

EDIT: we're adding the following .nsh script using electronBuilderConfig->nsis->include:

!macro customRemoveFiles
    DetailPrint "Removing files..."
    RMDir /r $INSTDIR
 !macroend

AviVahl commented 1 year ago

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 probably reached the 256 limit when trying to rename it and failed.

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.

cylon3035 commented 1 year ago

It is possible to get same error during first clean installation if install path + file path in the installation is greater than 256. First installer unzips files into temp directory then copies to install location. If it fails then error XYZ cannot be closed. Please close it manually and click retry to continue. will be displayed even thought XYZ was never installed before.

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. Different users have different user names or longer default install locations. It explains magic nature of reproducing this bug.

AviVahl commented 1 year ago

Yes, the error message is incorrect. In our case it was too long file paths.

sergeushenecz commented 12 months ago

Any workarounds exists? Now i downgrade to 22.14.13 and it works.

JumpinHack commented 11 months ago

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). We don't have long names, nor strange charsets (like chinese), and we hit the problem ONLY on Windows 11, on Windows 10 it' going as expected (is it possible that win11 has more restriction on path length than win10???)

nitsir commented 10 months ago

Same issue. Fixed with CRCCheck off https://github.com/electron-userland/electron-builder/issues/6409

flying19880517 commented 9 months ago

StrCpy and RM and other cmd not support long file path, un.atomicRMDir always failed. I'm also use "include" to use custom .nsh, but customRemoveFiles seems not run for my project, customInit works fine. Only delete Uninstall.exe can skip copy program files, and install success.

!macro customInit
  Delete "$INSTDIR\Uninstall*.exe"
!macroend

mirkin commented 8 months ago

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"

sergeushenecz commented 8 months ago

@mirkin The same. Only reverted to this version works as excepted.

VigneshSonaiya commented 8 months ago

I think this chunk of code was changed in this commit. I don't have knowledge on nsh script. maybe someone can check this. https://github.com/electron-userland/electron-builder/pull/6771/files

maoryadin commented 4 months ago

Weird, renaming the file to a really short name, make it work. No changes needed

felippenardi commented 3 months ago

I'm also having this problem, we have had no updates yet?

mmaietta commented 3 months ago

I need help recreating this issue via a minimum reproducible repo, then I can debug into this Ref: https://github.com/electron-userland/electron-builder/issues/6865#issuecomment-1468343573

Ali-Albaker commented 1 month ago

This is still an issue

ObadaZay commented 1 month ago

StrCpy and RM and other cmd not support long file path, un.atomicRMDir always failed. I'm also use "include" to use custom .nsh, but customRemoveFiles seems not run for my project, customInit works fine. Only delete Uninstall.exe can skip copy program files, and install success.
!macro customInit
  Delete "$INSTDIR\Uninstall*.exe"
!macroend

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

finnlaymfarmor commented 2 weeks ago

Also having these issues with 25.0.5, seriously breaking.

finnlaymfarmor commented 2 weeks ago

StrCpy and RM and other cmd not support long file path, un.atomicRMDir always failed. I'm also use "include" to use custom .nsh, but customRemoveFiles seems not run for my project, customInit works fine. Only delete Uninstall.exe can skip copy program files, and install success.
!macro customInit
  Delete "$INSTDIR\Uninstall*.exe"
!macroend
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

Will report back on how this goes

electron-userland / electron-builder

NSIS installer always show "XYZ cannot be closed. Please close it ..." when there is other process name contains "XYZ" #6865

4898 If the path contains Chinese characters, packaging will fail.

6232 shortcutName in chinese

6259 Random code After packing

A possible issue

A patch