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

install: Enhance windows installer - detect running gridcoinresearch(d).exe and ask to close before continuing #2672

Conversation

jamescowens
Copy link
Member

@jamescowens jamescowens commented Mar 31, 2023

Detect and prompt for shutdown of Gridcoin wallet during install for Windows

This commit introduces the use of ExecWait in combination with tasklist and find to detect a running Gridcoin wallet, either GUI or daemon, and prompt to shut it down before installation proceeds.

This method does not require the use of NSIS plugins, but has the disadvantage that two command windows momentarily appear during the check (one for gridcoinresearch.exe and the other for gridcoinresearchd.exe).

Closes #2662.

@barton2526
Copy link
Member

We are willingly diverging from Bitcoin here. I think this needs some extra scrutiny...

@jamescowens
Copy link
Member Author

Scrutinize away. This PR is trivial and essentially just detects whether an instance of Gridcoin is running and prompts the user to shut it down before continuing.

While we follow Bitcoin Core on many things, it doesn't mean that we are bound to stupid decisions that they make. Their installer should be doing this too.

@jamescowens jamescowens requested a review from div72 April 1, 2023 14:11
@div72
Copy link
Member

div72 commented Apr 1, 2023

Approach NACK, we should not be adding binary blobs even if only to the installer.

@jamescowens
Copy link
Member Author

jamescowens commented Apr 1, 2023

Then we need to ensure the module is installed on the build machines for the Windows build as an alternative approach. That will work just fine. Nsprocess is a very commonly used module for this purpose and is usually available as part of the NSIS install set, although it may be in a separate package. It is on OpenSUSE. I will check on our Ubuntu OS versions we use for the build environment.

@div72
Copy link
Member

div72 commented Apr 1, 2023

Eh, that can do. Alternatively, https://nsis.sourceforge.io/Check_whether_your_application_is_running also lists a way to do it without plugins by using tasklist and ExecWait.

@jamescowens
Copy link
Member Author

That was the very page I referenced to do this quick PR. The thing I dislike about that method is the external exec call to the tasklist. If we think that is preferable it wouldn't be too hard to use that instead. We don't support anything less than Windows 7, so it should be available for every Windows version we support.

@jamescowens
Copy link
Member Author

Actually, the ExecCmd plugin has been superseded by the ExecDos plugin. I have to check whether this plugin is provided in the default NSIS packages.

@jamescowens
Copy link
Member Author

The ExecDos plugin is not installed by the base NSIS package on Ubuntu. So that makes that approach equivalent in the packaging problem to my original approach.

@div72
Copy link
Member

div72 commented Apr 1, 2023

Wouldn't the ExecWait command work?

@jamescowens
Copy link
Member Author

jamescowens commented Apr 1, 2023

That will cause a command window to flash I think, which is definitely "hacky" and will raise questions.

@jamescowens
Copy link
Member Author

I am going to try ExecWait, but if it repeatedly flashes command windows, I am not going to use that.

@jamescowens
Copy link
Member Author

BTW this is a typical example of a BS change that is taking up an enormous amount of time. I may just close it. :|.

@jamescowens
Copy link
Member Author

jamescowens commented Apr 2, 2023

Hmm. I have fiddled around for several hours and cannot get ExecWait to work reliably.

This in a windows cmd does...

>"%SystemRoot%\System32\tasklist.exe" /NH /FI "IMAGENAME eq gridcoinresearch.exe" | "%SystemRoot%\System32\find.exe" /I "gridcoinresearch.exe"
gridcoinresearch.exe         32604 Console                    1  1,226,664 K

and then

>echo %errorlevel%
0

putting a purposefully non-matching exe name in the find part returns nothing and the %errorlevel% is then 1.

This part of the NSIS script should work then...

    loop:
    ExecWait '"%SystemRoot%\System32\tasklist.exe" /NH /FI \"IMAGENAME eq @GRIDCOIN_GUI_NAME@@EXEEXT@\" | "%SystemRoot%\System32\find.exe" /I \"@GRIDCOIN_GUI_NAME@@EXEEXT@\"' $0
    StrCmp $0 0 0 notRunning_gui
    MessageBox MB_OKCANCEL|MB_ICONEXCLAMATION 'The Gridcoin wallet (gridcoinresearch.exe) is running. Please close before continuing.' IDOK loop IDCANCEL end
    notRunning_gui:
    ExecWait '"%SystemRoot%\System32\tasklist.exe" /NH /FI \"IMAGENAME eq @GRIDCOIN_DAEMON_NAME@@EXEEXT@\" | "%SystemRoot%\System32\find.exe" /I \"@GRIDCOIN_DAEMON_NAME@@EXEEXT@\"' $0
    StrCmp $0 0 0 notRunning_daemon
    MessageBox MB_OKCANCEL|MB_ICONEXCLAMATION 'The Gridcoin wallet daemon (gridcoinresearchd.exe) is running. Please close before continuing.' IDOK loop IDCANCEL end
    notRunning_daemon:

But it doesn't. The ExecWaits appear to be doing nothing.

@jamescowens
Copy link
Member Author

jamescowens commented Apr 2, 2023

The source code for the nsProcess plugin is difficult to find. OpenSUSE has it for their OBS mingw packages, and I got it from there and can compile, but reliably downloading from source in a depends package for example is going to be difficult.

@jamescowens jamescowens self-assigned this Apr 2, 2023
…Windows

This commit introduces the use of ExecWait in combination with tasklist
and find to detect a running Gridcoin wallet, either GUI or daemon, and
prompt to shut it down before installation proceeds.
@jamescowens jamescowens force-pushed the fix_windows_installer_detect_running_gridcoin branch from a45a2cd to 888e23f Compare April 2, 2023 06:00
@jamescowens
Copy link
Member Author

Ok. I got it to work after fiddling around with ExecWait for hours with magic incantations on the quoting... :)

@jamescowens jamescowens changed the title install: Fix windows installer - detect running gridcoinresearch(d).exe and ask to end before continuing install: Fix windows installer - detect running gridcoinresearch(d).exe and ask to close before continuing Apr 2, 2023
@jamescowens jamescowens added this to the Miss Piggy milestone Apr 2, 2023
@jamescowens jamescowens changed the title install: Fix windows installer - detect running gridcoinresearch(d).exe and ask to close before continuing install: Enhance windows installer - detect running gridcoinresearch(d).exe and ask to close before continuing Apr 2, 2023
Copy link
Member

@div72 div72 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested ACK, seems to not work under Wine as tasklist is a stub but works on Windows.

@jamescowens
Copy link
Member Author

Yeah. That doesn't surprise me and I think we can ignore that corner case. Running a wallet under WINE on Linux works of course but is more of a stunt or purely for testing of a Windows binary on a Linux machine.

@jamescowens jamescowens merged commit d9541af into gridcoin-community:hotfix Apr 2, 2023
Copy link
Member

@barton2526 barton2526 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

post merge TACK

jamescowens added a commit to jamescowens/Gridcoin-Research that referenced this pull request Apr 5, 2023
Added
 - install: Enhance windows installer - detect running gridcoinresearch(d).exe and ask to close before continuing gridcoin-community#2672 (@jamescowens)
 - gui: Add one minute QTimer to update beacon age/expiration in tooltip gridcoin-community#2671 (@jamescowens)

Changed
none

Removed
none

Fixed
 - util: Implement workaround for backupwallet to deal with Boost 1.74 regression on copy_file gridcoin-community#2669 (@jamescowens)
 - banman: use GetPerformanceCounter instead of GetRandBytes gridcoin-community#2668 (@div72)
@jamescowens jamescowens deleted the fix_windows_installer_detect_running_gridcoin branch July 21, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows installer for 5.4.2.0-leisure doesn't close running client before attempting upgrade
3 participants