fix: use portable C++ RNG

I used "A Tour of C++, Third Edition" (ISBN-10 0136816487) as inspiration for the Random class.
This replaces all occurrences of `rand()` and `srand()`.

Co-authored-by: John Ericson <[email protected]>

src/libmain/shared.cc

@@ -174,15 +174,6 @@ void initNix(bool loadConfig)
        everybody. */
     umask(0022);
 
-    /* Initialise the PRNG. */
-    struct timeval tv;
-    gettimeofday(&tv, 0);
-#ifndef _WIN32
-    srandom(tv.tv_usec);
-#endif
-    srand(tv.tv_usec);
-
-
 }
 
 

src/libstore/build/derivation-goal.cc

@@ -17,6 +17,7 @@
 #include "topo-sort.hh"
 #include "callback.hh"
 #include "local-store.hh" // TODO remove, along with remaining downcasts
+#include "rng.hh"
 
 #include <regex>
 #include <queue>
@@ -765,64 +766,6 @@ Goal::Co DerivationGoal::tryLocalBuild() {
 }
 
 
-static void chmod_(const Path & path, mode_t mode)
-{
-    if (chmod(path.c_str(), mode) == -1)
-        throw SysError("setting permissions on '%s'", path);
-}
-
-
-/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if
-   it's a directory and we're not root (to be able to update the
-   directory's parent link ".."). */
-static void movePath(const Path & src, const Path & dst)
-{
-    auto st = lstat(src);
-
-    bool changePerm = (
-#ifndef _WIN32
-        geteuid()
-#else
-        !isRootUser()
-#endif
-        && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR));
-
-    if (changePerm)
-        chmod_(src, st.st_mode | S_IWUSR);
-
-    std::filesystem::rename(src, dst);
-
-    if (changePerm)
-        chmod_(dst, st.st_mode);
-}
-
-
-void replaceValidPath(const Path & storePath, const Path & tmpPath)
-{
-    /* We can't atomically replace storePath (the original) with
-       tmpPath (the replacement), so we have to move it out of the
-       way first.  We'd better not be interrupted here, because if
-       we're repairing (say) Glibc, we end up with a broken system. */
-    Path oldPath = fmt("%1%.old-%2%-%3%", storePath, getpid(), rand());
-    if (pathExists(storePath))
-        movePath(storePath, oldPath);
-
-    try {
-        movePath(tmpPath, storePath);
-    } catch (...) {
-        try {
-            // attempt to recover
-            movePath(oldPath, storePath);
-        } catch (...) {
-            ignoreException();
-        }
-        throw;
-    }
-
-    deletePath(oldPath);
-}
-
-
 int DerivationGoal::getChildStatus()
 {
 #ifndef _WIN32 // TODO enable build hook on Windows

src/libstore/filetransfer.cc

@@ -7,6 +7,7 @@
 #include "finally.hh"
 #include "callback.hh"
 #include "signals.hh"
+#include "rng.hh"
 
 #if ENABLE_S3
 #include <aws/core/client/ClientConfiguration.h>
@@ -26,7 +27,6 @@
 #include <cstring>
 #include <iostream>
 #include <queue>
-#include <random>
 #include <thread>
 #include <regex>
 
@@ -42,8 +42,7 @@ struct curlFileTransfer : public FileTransfer
 {
     CURLM * curlm = 0;
 
-    std::random_device rd;
-    std::mt19937 mt19937;
+    RandomFloatGenerator rng{0.0, 1.0};
 
     struct TransferItem : public std::enable_shared_from_this<TransferItem>
     {
@@ -502,7 +501,7 @@ struct curlFileTransfer : public FileTransfer
                         || writtenToSink == 0
                         || (acceptRanges && encoding.empty())))
                 {
-                    int ms = request.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + std::uniform_real_distribution<>(0.0, 0.5)(fileTransfer.mt19937));
+                    int ms = request.baseRetryTimeMs * std::pow(2.0f, attempt - 1 + fileTransfer.rng());
                     if (writtenToSink)
                         warn("%s; retrying from offset %d in %d ms", exc.what(), writtenToSink, ms);
                     else
@@ -539,7 +538,6 @@ struct curlFileTransfer : public FileTransfer
     std::thread workerThread;
 
     curlFileTransfer()
-        : mt19937(rd())
     {
         static std::once_flag globalInit;
         std::call_once(globalInit, curl_global_init, CURL_GLOBAL_ALL);

src/libstore/indirect-root-store.cc

@@ -8,7 +8,7 @@ void IndirectRootStore::makeSymlink(const Path & link, const Path & target)
     createDirs(dirOf(link));
 
     /* Create the new symlink. */
-    Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), rand());
+    Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), rng());
     createSymlink(target, tempLink);
 
     /* Atomically replace the old one. */

src/libstore/indirect-root-store.hh

@@ -2,6 +2,7 @@
 ///@file
 
 #include "local-fs-store.hh"
+#include "rng.hh"
 
 namespace nix {
 
@@ -68,6 +69,8 @@ struct IndirectRootStore : public virtual LocalFSStore
      */
     virtual void addIndirectRoot(const Path & path) = 0;
 
+    RandomIntGenerator rng{};
+
 protected:
     void makeSymlink(const Path & link, const Path & target);
 };

src/libstore/optimise-store.cc

@@ -222,7 +222,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
        its timestamp back to 0. */
     MakeReadOnly makeReadOnly(mustToggle ? dirOfPath : "");
 
-    std::filesystem::path tempLink = fmt("%1%/.tmp-link-%2%-%3%", realStoreDir, getpid(), rand());
+    std::filesystem::path tempLink = fmt("%1%/.tmp-link-%2%-%3%", realStoreDir, getpid(), rng());
 
     try {
         std::filesystem::create_hard_link(linkPath, tempLink);

src/libstore/sqlite.cc

@@ -3,6 +3,7 @@
 #include "util.hh"
 #include "url.hh"
 #include "signals.hh"
+#include "rng.hh"
 
 #include <sqlite3.h>
 
@@ -258,7 +259,7 @@ void handleSQLiteBusy(const SQLiteBusy & e, time_t & nextWarning)
        is likely to fail again. */
     checkInterrupt();
     /* <= 0.1s */
-    std::this_thread::sleep_for(std::chrono::milliseconds { rand() % 100 });
+    std::this_thread::sleep_for(std::chrono::milliseconds { RandomIntGenerator{0, 100}() });
 }
 
 }

src/libstore/unix/build/local-derivation-goal.cc

@@ -267,17 +267,14 @@ Goal::Co LocalDerivationGoal::tryLocalBuild()
     co_return buildDone();
 }
 
-static void chmod_(const Path & path, mode_t mode)
+void LocalDerivationGoal::chmod_(const Path & path, mode_t mode)
 {
     if (chmod(path.c_str(), mode) == -1)
         throw SysError("setting permissions on '%s'", path);
 }
 
 
-/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if
-   it's a directory and we're not root (to be able to update the
-   directory's parent link ".."). */
-static void movePath(const Path & src, const Path & dst)
+void LocalDerivationGoal::movePath(const Path & src, const Path & dst)
 {
     auto st = lstat(src);
 
@@ -293,7 +290,30 @@ static void movePath(const Path & src, const Path & dst)
 }
 
 
-extern void replaceValidPath(const Path & storePath, const Path & tmpPath);
+void LocalDerivationGoal::replaceValidPath(const Path & storePath, const Path & tmpPath)
+{
+    /* We can't atomically replace storePath (the original) with
+       tmpPath (the replacement), so we have to move it out of the
+       way first.  We'd better not be interrupted here, because if
+       we're repairing (say) Glibc, we end up with a broken system. */
+    Path oldPath = fmt("%1%.old-%2%-%3%", storePath, getpid(), getLocalStore().rng());
+    if (pathExists(storePath))
+        movePath(storePath, oldPath);
+
+    try {
+        movePath(tmpPath, storePath);
+    } catch (...) {
+        try {
+            // attempt to recover
+            movePath(oldPath, storePath);
+        } catch (...) {
+            ignoreException();
+        }
+        throw;
+    }
+
+    deletePath(oldPath);
+}
 
 
 int LocalDerivationGoal::getChildStatus()

src/libstore/unix/build/local-derivation-goal.hh

@@ -307,6 +307,16 @@ struct LocalDerivationGoal : public DerivationGoal
      * rewrites caught everything
      */
     StorePath makeFallbackPath(OutputNameView outputName);
+
+    void chmod_(const Path & path, mode_t mode);
+
+    /**
+     * Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if
+     * it's a directory and we're not root (to be able to update the
+     * directory's parent link "..").
+     */
+    void movePath(const Path & src, const Path & dst);
+    void replaceValidPath(const Path & storePath, const Path & tmpPath);
 };
 
 }

src/libutil/meson.build

@@ -248,6 +248,7 @@ headers = [config_h] + files(
   'util.hh',
   'variant-wrapper.hh',
   'xml-writer.hh',
+  'rng.hh'
 )
 
 if host_machine.system() == 'linux'

src/libutil/rng.hh

@@ -0,0 +1,34 @@
+#pragma once
+
+#include <random>
+#include <limits>
+
+namespace nix {
+
+// Inspired by the book "A Tour of C++, Third Edition" (ISBN-10 0136816487)
+template<typename T, typename Distribution, typename Engine>
+struct RandomNumberGenerator
+{
+public:
+    using limits = std::numeric_limits<T>;
+    RandomNumberGenerator(T low = limits::min(), T high = limits::max())
+        : engine(std::random_device{}())
+        , dist(low, high){};
+    T operator()()
+    {
+        return dist(engine);
+    }
+    void seed(int s)
+    {
+        engine.seed(s);
+    }
+private:
+    Engine engine;
+    Distribution dist;
+};
+
+using RandomIntGenerator = RandomNumberGenerator<int, std::uniform_int_distribution<int>, std::default_random_engine>;
+using RandomFloatGenerator =
+    RandomNumberGenerator<float, std::uniform_real_distribution<float>, std::default_random_engine>;
+
+} // namespace nix