From 8f6ab318635d823a7e6ad9020ac17b8d2dd0e91b Mon Sep 17 00:00:00 2001
From: willcl-ark <will@256k1.dev>
Date: Mon, 27 Nov 2023 10:29:02 +0000
Subject: [PATCH] init: don't delete PID file if it was not generated

Previously, starting a second bitcoind using the same datadir would
correctly fail to init and shutdown. However during shutdown the PID
file belonging to the first instance would be erroneously removed by
the second process shutting down.

Fix this to only delete the PID file if we created it.
---
 src/init.cpp                        | 25 ++++++++++++++++++-------
 test/functional/feature_filelock.py |  3 +++
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index 526a8d62711eb..ac52b34fc5c56 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -155,6 +155,11 @@ static const char* DEFAULT_ASMAP_FILENAME="ip_asn.map";
  * The PID file facilities.
  */
 static const char* BITCOIN_PID_FILENAME = "bitcoind.pid";
+/**
+ * True if this process has created a PID file.
+ * Used to determine whether we should remove the PID file on shutdown.
+ */
+static bool g_generated_pid{false};
 
 static fs::path GetPidFile(const ArgsManager& args)
 {
@@ -170,12 +175,24 @@ static fs::path GetPidFile(const ArgsManager& args)
 #else
         tfm::format(file, "%d\n", getpid());
 #endif
+        g_generated_pid = true;
         return true;
     } else {
         return InitError(strprintf(_("Unable to create the PID file '%s': %s"), fs::PathToString(GetPidFile(args)), SysErrorString(errno)));
     }
 }
 
+static void RemovePidFile(const ArgsManager& args)
+{
+    if (!g_generated_pid) return;
+    const auto pid_path{GetPidFile(args)};
+    if (std::error_code error; !fs::remove(pid_path, error)) {
+        std::string msg{error ? error.message() : "File does not exist"};
+        LogPrintf("Unable to remove PID file (%s): %s\n", fs::PathToString(pid_path), msg);
+    }
+}
+
+
 //////////////////////////////////////////////////////////////////////////////
 //
 // Shutdown
@@ -352,13 +369,7 @@ void Shutdown(NodeContext& node)
     node.scheduler.reset();
     node.kernel.reset();
 
-    try {
-        if (!fs::remove(GetPidFile(*node.args))) {
-            LogPrintf("%s: Unable to remove PID file: File does not exist\n", __func__);
-        }
-    } catch (const fs::filesystem_error& e) {
-        LogPrintf("%s: Unable to remove PID file: %s\n", __func__, fsbridge::get_filesystem_error_message(e));
-    }
+    RemovePidFile(*node.args);
 
     LogPrintf("%s: done\n", __func__);
 }
diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py
index 0f4be54d0e24a..567207915e84b 100755
--- a/test/functional/feature_filelock.py
+++ b/test/functional/feature_filelock.py
@@ -30,8 +30,11 @@ def run_test(self):
         expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running."
         self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg)
 
+        self.log.info("Check that cookie and PID file are not deleted when attempting to start a second bitcoind using the same datadir")
         cookie_file = datadir / ".cookie"
         assert cookie_file.exists()  # should not be deleted during the second bitcoind instance shutdown
+        pid_file = datadir / "bitcoind.pid"
+        assert pid_file.exists()
 
         if self.is_wallet_compiled():
             def check_wallet_filelock(descriptors):