riordant commented at 11:47 AM on February 21, 2019: contributor

Introduction

As discussed with @laanwj on IRC:

PID file creation was never enabled for Windows, as the pid_t filetype is not available for it. However, the WIN32 API contains the header Processthreadsapi.h which in turn contains the function GetCurrentProcessId(). ~~This function is called at a higher level by _getpid()~~ EDIT: _getpid() is not available to the MSVC compiler used in the AppVeyor build. As a result, I have changed the function call toGetCurrentProcessId(), which performs the same function and is available to both MinGW & MSVC. This allows one to capture the PID in Windows, without any additional includes - the above function is already available.
Within this PR, I have added a separate line that calls GetCurrentProcessId() in the case of a WIN compilation, and the usual getpid() otherwise. All code blocks processing PID file logic that avoid WIN32 have been changed to consider it. I have also updated the preprocessor definitions in libbitcoin_server.vcxproj.in to suppress a warning related to std::strerror for the MSVC build, that was causing the AppVeyor build to fail (see @fanquake comment below).

Rationale

Consistency between OS's running Bitcoin
- Applications which build off of bitcoind, such as novel front-end clients, often need access to the PID in order to control the daemon. Instead of designing some alternate way of doing this for one system, it should be consistent between all of them.

In collaboration with @joernroeder

fanquake added the label Windows on Feb 21, 2019

laanwj commented at 12:02 PM on February 21, 2019: member

+4 -10 and adds a feature, this is a great first contribution, thanks! utACK ca33ac10d36199395bd6553db77e5ceacfdb925e (given that this passes Travis and Appveyor)

riordant commented at 12:29 PM on February 21, 2019: contributor

Force pushed in order to add co-author.

fanquake requested review from ken2812221 on Feb 21, 2019

ken2812221 approved

ken2812221 commented at 2:31 PM on February 21, 2019: contributor

~utACK c4372b8b18d2fab2101d904289b78f87703bb6b9~ You might have to include process.h?

fanquake commented at 1:21 AM on February 22, 2019: member

Appveyor build is failing with:

c:\projects\bitcoin\src\init.cpp(112): error C3861: '_getpid': identifier not found [C:\projects\bitcoin\build_msvc\libbitcoin_server\libbitcoin_server.vcxproj]

c:\projects\bitcoin\src\init.cpp(119): error C4996: 'strerror': This function or variable may be unsafe. Consider using strerror_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [C:\projects\bitcoin\build_msvc\libbitcoin_server\libbitcoin_server.vcxproj]

fanquake commented at 1:38 AM on February 22, 2019: member

I compiled this PR inside WSL and ran bitcoin-qt on a Windows 10 system. Looks like bitcoin.pid is created correctly, and contains the correct PID.

windows

riordant commented at 7:51 AM on February 22, 2019: contributor

Update: AppVeyor build should now pass. I have updated the description with details

in src/init.cpp:296 in 4eb50777a1 outdated

 292 |      try {
 293 |          if (!fs::remove(GetPidFile())) {
 294 |              LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__);
 295 |          }
 296 |      } catch (const fs::filesystem_error& e) {
 297 |          LogPrintf("%s: Unable to remove PID file: %s\n", __func__, e.what());

ken2812221 commented at 12:44 PM on February 22, 2019:

nit: Use fsbridge::get_filesystem_error_message(e) instead of e.what()

riordant commented at 8:47 AM on February 23, 2019:

Ok, can be added. Just note this was not added/modified by me and was merged in #15278 a few days ago

ken2812221 commented at 7:49 PM on February 24, 2019:

That was OK to use e.what() in other OS except Windows. So you have to use fsbridge::get_filesystem_error_message(e) instead.

practicalswift commented at 7:34 PM on February 24, 2019: contributor

Concept ACK

Excellent first-time contribution! Thanks! Hope to see more PR:s from you! :-)

hebasto commented at 8:47 PM on February 24, 2019: member

Concept ACK.

laanwj commented at 8:35 AM on February 25, 2019: member

utACK after squash, I think this is better as one commit to have an atomic change

Enable PID file creation on Windows

- Add available WIN PID function
- Consider WIN32 in each relevant case
- Add new preprocessor definitions to suppress warning
- Update error message for generic OS

Co-authored-by: Jörn Röder <kontakt@joernroeder.de>

3f5ad622e5

riordant commented at 9:10 AM on February 25, 2019: contributor

Yep agree, done.

laanwj commented at 12:07 PM on February 25, 2019: member

thanks! utACK 3f5ad622e5fe0781a70bee9e3322b23c2352e956

laanwj merged this on Feb 25, 2019

laanwj closed this on Feb 25, 2019

laanwj referenced this in commit b4fc5257b7 on Feb 25, 2019

deadalnix referenced this in commit 49cd702189 on Apr 20, 2020

ftrader referenced this in commit 6b6ca64094 on Aug 17, 2020

Munkybooty referenced this in commit 396e3094fd on Sep 7, 2021

6293 referenced this in commit 938e87c9ba on Nov 27, 2021

6293 referenced this in commit 2cb47e4f12 on Nov 27, 2021

6293 referenced this in commit 0da17d28de on Nov 27, 2021

DrahtBot locked this on Dec 16, 2021

Enable PID file creation on WIN #15456

Introduction

Rationale