Update IceUtilInternal::Random implementation to use std::random

cpp/include/IceUtil/Random.h

@@ -6,7 +6,6 @@
 #define ICE_UTIL_RANDOM_H
 
 #include <IceUtil/Config.h>
-#include <IceUtil/Exception.h>
 
 #include <algorithm>
 #include <random>
@@ -15,13 +14,13 @@ namespace IceUtilInternal
 {
 
 ICE_API void generateRandom(char*, size_t);
-ICE_API unsigned int random(int = 0);
+ICE_API unsigned int random(unsigned int = 0);
 
 template<class T>
 void shuffle(T first, T last)
 {
-    std::random_device rd;
-    std::mt19937 rng(rd());
+    thread_local static std::random_device rd;
+    thread_local static std::mt19937 rng(rd());
     std::shuffle(first, last, rng);
 }
 

cpp/src/IceGrid/Database.cpp

@@ -1755,7 +1755,7 @@ Database::getObjectByType(const string& type, const shared_ptr<Ice::Connection>&
     {
         return 0;
     }
-    return objs[IceUtilInternal::random(static_cast<int>(objs.size()))];
+    return objs[IceUtilInternal::random(static_cast<unsigned int>(objs.size()))];
 }
 
 shared_ptr<Ice::ObjectPrx>

cpp/src/IceUtil/Random.cpp

@@ -2,172 +2,28 @@
 // Copyright (c) ZeroC, Inc. All rights reserved.
 //
 
-#ifdef _WIN32
-#  define _CRT_RAND_S
-#endif
-
 #include <IceUtil/Random.h>
 
-#include <mutex>
-
-#ifndef _WIN32
-#   include <unistd.h>
-#   include <fcntl.h>
-#endif
-
 using namespace std;
-using namespace IceUtil;
-
-#if !defined(_WIN32)
-namespace
-{
-
-//
-// The static mutex is required to lazy initialize the file
-// descriptor for /dev/urandom (Unix)
-//
-// Also, unfortunately on Linux (at least up to 2.6.9), concurrent
-// access to /dev/urandom can return the same value. Search for
-// "Concurrent access to /dev/urandom" in the linux-kernel mailing
-// list archive for additional details.  Since /dev/urandom on other
-// platforms is usually a port from Linux, this problem could be
-// widespread. Therefore, we serialize access to /dev/urandom using a
-// static mutex.
-//
-mutex staticMutex;
-int fd = -1;
-
-//
-// Callback to use with pthread_atfork to reset the "/dev/urandom"
-// fd state. We don't need to close the fd here as that is done
-// during static destruction.
-//
-extern "C"
-{
-
-void childAtFork()
-{
-    if(fd != -1)
-    {
-        fd = -1;
-    }
-}
-
-}
-
-class Init
-{
-public:
-
-    Init()
-    {
-        // Register a callback to reset the "/dev/urandom" fd state after fork.
-        pthread_atfork(0, 0, &childAtFork);
-    }
-
-    ~Init()
-    {
-        if(fd != -1)
-        {
-            close(fd);
-            fd = -1;
-        }
-    }
-};
-
-Init init;
-
-}
-#endif
 
 void
 IceUtilInternal::generateRandom(char* buffer, size_t size)
 {
-#ifdef _WIN32
-    int i = 0;
-    const size_t randSize = sizeof(unsigned int);
-
-    while(size - i >= randSize)
-    {
-        unsigned int r = 0;
-        errno_t err = rand_s(&r);
-        if(err != 0)
-        {
-            throw SyscallException(__FILE__, __LINE__, errno);
-        }
-        memcpy(buffer + i, &r, randSize);
-        i += randSize;
-    }
-
-    if(size - i > 0)
-    {
-        assert(size - i < randSize);
-        unsigned int r = 0;
-        errno_t err = rand_s(&r);
-        if(err != 0)
-        {
-            throw SyscallException(__FILE__, __LINE__, errno);
-        }
-        memcpy(buffer + i, &r, size - i);
-    }
-#else
-    //
-    // Serialize access to /dev/urandom; see comment above.
-    //
-    lock_guard lock(staticMutex);
-    if(fd == -1)
-    {
-        fd = open("/dev/urandom", O_RDONLY);
-        if(fd == -1)
-        {
-            throw SyscallException(__FILE__, __LINE__, errno);
-        }
-    }
-
-    //
-    // Limit the number of attempts to 20 reads to avoid
-    // a potential "for ever" loop
-    //
-    int reads = 0;
-    size_t index = 0;
-    while(reads <= 20 && index != size)
-    {
-        ssize_t bytesRead = read(fd, buffer + index, size - index);
-
-        if(bytesRead == -1 && errno != EINTR)
-        {
-            throw SyscallException(__FILE__, __LINE__, errno);
-        }
-        else
-        {
-            index += static_cast<size_t>(bytesRead);
-            reads++;
-        }
-    }
-
-    if(index != size)
+    // We use the random_device directly here to get cryptographic random numbers when possible.
+    thread_local static std::random_device rd;
+    uniform_int_distribution<unsigned int> distribution(0, 255);
+    for (size_t i = 0; i < size; ++i, ++buffer)
     {
-        throw SyscallException(__FILE__, __LINE__, 0);
+        *buffer = static_cast<char>(distribution(rd));
     }
-#endif
 }
 
 unsigned int
-IceUtilInternal::random(int limit)
+IceUtilInternal::random(unsigned int limit)
 {
-    unsigned int r;
-#ifdef _WIN32
-    errno_t err = rand_s(&r);
-    if(err != 0)
-    {
-        throw SyscallException(__FILE__, __LINE__, errno);
-    }
-#else
-    generateRandom(reinterpret_cast<char*>(&r), sizeof(unsigned int));
-#endif
-    if(limit > 0)
-    {
-        r = r % static_cast<unsigned int>(limit);
-    }
-    return r;
+    assert(limit > 0);
+    thread_local static std::random_device rd;
+    thread_local static std::mt19937 rng(rd());
+    uniform_int_distribution<unsigned int> distribution(0, limit - 1);
+    return distribution(rng);
 }

cpp/test/Ice/binding/AllTests.cpp

@@ -249,14 +249,14 @@ allTests(Test::TestHelper* helper)
             for(i = 0; i < proxies.size(); ++i)
             {
                 vector<RemoteObjectAdapterPrxPtr> adpts;
-                adpts.resize(IceUtilInternal::random(static_cast<int>(adapters.size())));
+                adpts.resize(IceUtilInternal::random(static_cast<unsigned int>(adapters.size())));
                 if(adpts.empty())
                 {
                     adpts.resize(1);
                 }
                 for(vector<RemoteObjectAdapterPrxPtr>::iterator p = adpts.begin(); p != adpts.end(); ++p)
                 {
-                    *p = adapters[IceUtilInternal::random(static_cast<int>(adapters.size()))];
+                    *p = adapters[IceUtilInternal::random(static_cast<unsigned int>(adapters.size()))];
                 }
                 proxies[i] = createTestIntfPrx(adpts);
             }