From 143a118fbf16feaae68dc5d4174f8582c9be7568 Mon Sep 17 00:00:00 2001
From: Philipp Moritz <pcmoritz@gmail.com>
Date: Thu, 9 Aug 2018 15:37:17 -0700
Subject: [PATCH] [xray] Fix valgrind crash when memory profiling raylet
 (#2583)

* use different random number generator to be compatible with older valgrind versions

* seed from time

* style

* fix

* remove more random devices

* also remove random_device from global scheduler

* rename mutex

* linting
---
 .../global_scheduler_algorithm.h              | 14 ++++++++++----
 src/ray/id.cc                                 | 19 +++++++++++++++++--
 src/ray/raylet/scheduling_policy.cc           |  5 ++++-
 src/ray/raylet/scheduling_policy.h            |  2 --
 4 files changed, 31 insertions(+), 9 deletions(-)

diff --git a/src/global_scheduler/global_scheduler_algorithm.h b/src/global_scheduler/global_scheduler_algorithm.h
index b249d84a7..69be67d97 100644
--- a/src/global_scheduler/global_scheduler_algorithm.h
+++ b/src/global_scheduler/global_scheduler_algorithm.h
@@ -1,6 +1,7 @@
 #ifndef GLOBAL_SCHEDULER_ALGORITHM_H
 #define GLOBAL_SCHEDULER_ALGORITHM_H
 
+#include <chrono>
 #include <random>
 
 #include "common.h"
@@ -25,9 +26,16 @@ enum class GlobalSchedulerAlgorithm {
 class GlobalSchedulerPolicyState {
  public:
   GlobalSchedulerPolicyState(int64_t round_robin_index)
-      : round_robin_index_(round_robin_index), gen_(rd_()) {}
+      : round_robin_index_(round_robin_index),
+        gen_(std::chrono::high_resolution_clock::now()
+                 .time_since_epoch()
+                 .count()) {}
 
-  GlobalSchedulerPolicyState() : round_robin_index_(0), gen_(rd_()) {}
+  GlobalSchedulerPolicyState()
+      : round_robin_index_(0),
+        gen_(std::chrono::high_resolution_clock::now()
+                 .time_since_epoch()
+                 .count()) {}
 
   /// Return the policy's random number generator.
   ///
@@ -42,8 +50,6 @@ class GlobalSchedulerPolicyState {
  private:
   /// The index of the next local scheduler to assign a task to.
   int64_t round_robin_index_;
-  /// Internally maintained random number engine device.
-  std::random_device rd_;
   /// Internally maintained random number generator.
   std::mt19937_64 gen_;
 };
diff --git a/src/ray/id.cc b/src/ray/id.cc
index f3fc7806c..01dbf4444 100644
--- a/src/ray/id.cc
+++ b/src/ray/id.cc
@@ -1,6 +1,9 @@
 #include "ray/id.h"
 
 #include <limits.h>
+
+#include <chrono>
+#include <mutex>
 #include <random>
 
 #include "ray/constants.h"
@@ -8,6 +11,12 @@
 
 namespace ray {
 
+std::mt19937 RandomlySeededMersenneTwister() {
+  auto seed = std::chrono::high_resolution_clock::now().time_since_epoch().count();
+  std::mt19937 seeded_engine(seed);
+  return seeded_engine;
+}
+
 UniqueID::UniqueID(const plasma::UniqueID &from) {
   std::memcpy(&id_, from.data(), kUniqueIDSize);
 }
@@ -15,9 +24,15 @@ UniqueID::UniqueID(const plasma::UniqueID &from) {
 UniqueID UniqueID::from_random() {
   UniqueID id;
   uint8_t *data = id.mutable_data();
-  std::random_device engine;
+  // NOTE(pcm): The right way to do this is to have one std::mt19937 per
+  // thread (using the thread_local keyword), but that's not supported on
+  // older versions of macOS (see https://stackoverflow.com/a/29929949)
+  static std::mutex random_engine_mutex;
+  std::lock_guard<std::mutex> lock(random_engine_mutex);
+  static std::mt19937 generator = RandomlySeededMersenneTwister();
+  std::uniform_int_distribution<uint32_t> dist(0, std::numeric_limits<uint8_t>::max());
   for (int i = 0; i < kUniqueIDSize; i++) {
-    data[i] = static_cast<uint8_t>(engine());
+    data[i] = static_cast<uint8_t>(dist(generator));
   }
   return id;
 }
diff --git a/src/ray/raylet/scheduling_policy.cc b/src/ray/raylet/scheduling_policy.cc
index 0ff0ee631..2cf739906 100644
--- a/src/ray/raylet/scheduling_policy.cc
+++ b/src/ray/raylet/scheduling_policy.cc
@@ -1,5 +1,7 @@
 #include "scheduling_policy.h"
 
+#include <chrono>
+
 #include "ray/util/logging.h"
 
 namespace ray {
@@ -7,7 +9,8 @@ namespace ray {
 namespace raylet {
 
 SchedulingPolicy::SchedulingPolicy(const SchedulingQueue &scheduling_queue)
-    : scheduling_queue_(scheduling_queue), gen_(rd_()) {}
+    : scheduling_queue_(scheduling_queue),
+      gen_(std::chrono::high_resolution_clock::now().time_since_epoch().count()) {}
 
 std::unordered_map<TaskID, ClientID> SchedulingPolicy::Schedule(
     const std::unordered_map<ClientID, SchedulingResources> &cluster_resources,
diff --git a/src/ray/raylet/scheduling_policy.h b/src/ray/raylet/scheduling_policy.h
index d89201c3b..89a725243 100644
--- a/src/ray/raylet/scheduling_policy.h
+++ b/src/ray/raylet/scheduling_policy.h
@@ -38,8 +38,6 @@ class SchedulingPolicy {
  private:
   /// An immutable reference to the scheduling task queues.
   const SchedulingQueue &scheduling_queue_;
-  /// Internally maintained random number engine device.
-  std::random_device rd_;
   /// Internally maintained random number generator.
   std::mt19937_64 gen_;
 };