Merge bitcoin#16300: util: Explain why the path is cached

fa69c3e util: Explain why the path is cached (MarcoFalke)

Pull request description:

  The rationale for caching the datadir is given as

  ```
      // This can be called during exceptions by LogPrintf(), so we cache the
      // value so we don't have to do memory allocations after that.
  ```

  Since 8c2d695, the debug log location is actually cached itself in `m_file_path`.

  So explain that the caching is now only used to guard against disk access on each call. (See also bitcoin#16255)

ACKs for top commit:
  promag:
    ACK fa69c3e.
  laanwj:
    ACK fa69c3e
  ryanofsky:
    utACK fa69c3e. Good cleanup. Previous comment was confusing, and definitely not helpful if outdated.

Tree-SHA512: 02108c90026d6d7c02843aaf59a06b4e1fa63d5d4378bb7760f50767efc340dc94c259bf7afb32fa4d47952b48a4e91798d1e0ddc1b051d770405e078636793a

src/util/system.cpp

@@ -768,19 +768,16 @@ fs::path GetDefaultDataDir()
 static fs::path g_blocks_path_cache_net_specific;
 static fs::path pathCached;
 static fs::path pathCachedNetSpecific;
-static CCriticalSection csPathCached;
+static RecursiveMutex csPathCached;
 
 const fs::path &GetBlocksDir()
 {
-
     LOCK(csPathCached);
-
     fs::path &path = g_blocks_path_cache_net_specific;
 
-    // This can be called during exceptions by LogPrintf(), so we cache the
-    // value so we don't have to do memory allocations after that.
-    if (!path.empty())
-        return path;
+    // Cache the path to avoid calling fs::create_directories on every call of
+    // this function
+    if (!path.empty()) return path;
 
     if (gArgs.IsArgSet("-blocksdir")) {
         path = fs::system_complete(gArgs.GetArg("-blocksdir", ""));
@@ -800,15 +797,12 @@ const fs::path &GetBlocksDir()
 
 const fs::path &GetDataDir(bool fNetSpecific)
 {
-
     LOCK(csPathCached);
-
     fs::path &path = fNetSpecific ? pathCachedNetSpecific : pathCached;
 
-    // This can be called during exceptions by LogPrintf(), so we cache the
-    // value so we don't have to do memory allocations after that.
-    if (!path.empty())
-        return path;
+    // Cache the path to avoid calling fs::create_directories on every call of
+    // this function
+    if (!path.empty()) return path;
 
     if (gArgs.IsArgSet("-datadir")) {
         path = fs::system_complete(gArgs.GetArg("-datadir", ""));

src/util/system.h

@@ -1,5 +1,5 @@
 // Copyright (c) 2009-2010 Satoshi Nakamoto
-// Copyright (c) 2009-2018 The Bitcoin Core developers
+// Copyright (c) 2009-2019 The Bitcoin Core developers
 // Copyright (c) 2014-2021 The Dash Core developers
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
@@ -21,20 +21,17 @@
 #include <fs.h>
 #include <logging.h>
 #include <sync.h>
-#include <util/threadnames.h>
 #include <tinyformat.h>
 #include <util/memory.h>
+#include <util/threadnames.h>
 #include <util/time.h>
 #include <amount.h>
 
-#include <atomic>
 #include <exception>
 #include <map>
 #include <set>
 #include <stdint.h>
 #include <string>
-#include <unordered_map>
-#include <unordered_set>
 #include <utility>
 #include <vector>
 
@@ -107,6 +104,7 @@ fs::path GetDefaultDataDir();
 const fs::path &GetBlocksDir();
 const fs::path &GetDataDir(bool fNetSpecific = true);
 fs::path GetBackupsDir();
+/** Tests only */
 void ClearDatadirCache();
 fs::path GetConfigFile(const std::string& confPath);
 #ifdef WIN32