From 51436f85a2bfcb7874a0c3b12add2df19ff18a3b Mon Sep 17 00:00:00 2001
From: TheCharlatan <seb.kung@gmail.com>
Date: Fri, 14 Feb 2025 22:20:54 +0100
Subject: [PATCH] init: Take lock on blocks directory in BlockManager ctor

This moves the responsibility of taking the lock for the blocks
directory into the BlockManager. Use the DirectoryLock wrapper to ensure
it is the first resource to be acquired and is released again after use.

This is relevant for the kernel library where the lock should be taken
even if the user fails to explicitly do so.
---
 src/init.cpp              | 14 +++-----------
 src/init/common.cpp       |  7 ++++---
 src/node/blockstorage.cpp |  4 +++-
 src/node/blockstorage.h   |  3 ++-
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index d46318fd45e..9346e9e2048 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1107,11 +1107,6 @@ static bool LockDirectory(const fs::path& dir, bool probeOnly)
     } // no default case, so the compiler can warn about missing cases
     assert(false);
 }
-static bool LockDirectories(bool probeOnly)
-{
-    return LockDirectory(gArgs.GetDataDirNet(), probeOnly) && \
-           LockDirectory(gArgs.GetBlocksDirPath(), probeOnly);
-}
 
 bool AppInitSanityChecks(const kernel::Context& kernel)
 {
@@ -1129,7 +1124,8 @@ bool AppInitSanityChecks(const kernel::Context& kernel)
     // Probe the directory locks to give an early error message, if possible
     // We cannot hold the directory locks here, as the forking for daemon() hasn't yet happened,
     // and a fork will cause weird behavior to them.
-    return LockDirectories(true);
+    return LockDirectory(gArgs.GetDataDirNet(), /*probeOnly=*/true)
+        && LockDirectory(gArgs.GetBlocksDirPath(), /*probeOnly=*/true);
 }
 
 bool AppInitLockDirectories()
@@ -1137,11 +1133,7 @@ bool AppInitLockDirectories()
     // After daemonization get the directory locks again and hold on to them until exit
     // This creates a slight window for a race condition to happen, however this condition is harmless: it
     // will at most make us exit without printing a message to console.
-    if (!LockDirectories(false)) {
-        // Detailed error printed inside LockDirectory
-        return false;
-    }
-    return true;
+    return LockDirectory(gArgs.GetDataDirNet(), /*probeOnly=*/false);
 }
 
 bool AppInitInterfaces(NodeContext& node)
diff --git a/src/init/common.cpp b/src/init/common.cpp
index bc0a2f65080..e90d34afb05 100644
--- a/src/init/common.cpp
+++ b/src/init/common.cpp
@@ -119,9 +119,10 @@ bool StartLogging(const ArgsManager& args)
     }
 
     if (!LogInstance().m_log_timestamps)
-        LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime()));
-    LogPrintf("Default data directory %s\n", fs::PathToString(GetDefaultDataDir()));
-    LogPrintf("Using data directory %s\n", fs::PathToString(gArgs.GetDataDirNet()));
+        LogInfo("Startup time: %s", FormatISO8601DateTime(GetTime()));
+    LogInfo("Default data directory %s", fs::PathToString(GetDefaultDataDir()));
+    LogInfo("Using data directory %s", fs::PathToString(gArgs.GetDataDirNet()));
+    LogInfo("Using blocks directory %s", fs::PathToString(gArgs.GetBlocksDirPath()));
 
     // Only log conf file usage message if conf file actually exists.
     fs::path config_file_path = args.GetConfigFilePath();
diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp
index 372395dd24d..e3209847556 100644
--- a/src/node/blockstorage.cpp
+++ b/src/node/blockstorage.cpp
@@ -31,6 +31,7 @@
 #include <util/batchpriority.h>
 #include <util/check.h>
 #include <util/fs.h>
+#include <util/fs_helpers.h>
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 #include <util/translation.h>
@@ -1165,7 +1166,8 @@ static auto InitBlocksdirXorKey(const BlockManager::Options& opts)
 }
 
 BlockManager::BlockManager(const util::SignalInterrupt& interrupt, Options opts)
-    : m_prune_mode{opts.prune_target > 0},
+    : m_blocks_dir_lock{DirectoryLock(opts.blocks_dir, "blocks")},
+      m_prune_mode{opts.prune_target > 0},
       m_xor_key{InitBlocksdirXorKey(opts)},
       m_opts{std::move(opts)},
       m_block_file_seq{FlatFileSeq{m_opts.blocks_dir, "blk", m_opts.fast_prune ? 0x4000 /* 16kB */ : BLOCKFILE_CHUNK_SIZE}},
diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h
index 8a34efadfea..5db9aa82d67 100644
--- a/src/node/blockstorage.h
+++ b/src/node/blockstorage.h
@@ -18,6 +18,7 @@
 #include <sync.h>
 #include <uint256.h>
 #include <util/fs.h>
+#include <util/fs_helpers.h>
 #include <util/hasher.h>
 
 #include <array>
@@ -127,7 +128,6 @@ struct BlockfileCursor {
 
 std::ostream& operator<<(std::ostream& os, const BlockfileCursor& cursor);
 
-
 /**
  * Maintains a tree of blocks (stored in `m_block_index`) which is consulted
  * to determine where the most-work tip is.
@@ -141,6 +141,7 @@ class BlockManager
     friend ChainstateManager;
 
 private:
+    DirectoryLock m_blocks_dir_lock;
     const CChainParams& GetParams() const { return m_opts.chainparams; }
     const Consensus::Params& GetConsensus() const { return m_opts.chainparams.GetConsensus(); }
     /**