From: Cameron Pickett <pickett@fb.com>
Date: Tue, 10 Oct 2017 09:39:20 +0000 (-0700)
Subject: Handle nullptr from getTimekeeperSingleton
X-Git-Tag: v2017.10.16.00~8
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=d1c5974be19dcdcbf4f490eabd994fdc78da6086;p=folly.git

Handle nullptr from getTimekeeperSingleton

Summary:
According to folly::Singleton::try_get(), https://fburl.com/23wqby9i, the caller is responsible for handling a nullptr return. In this case, we handle it poorly, via a crash both in production and debug code.

This diff opts for handling the nullptr more gracefully, via a future loaded with an exception.

Reviewed By: yfeldblum

Differential Revision: D6006864

fbshipit-source-id: e8fde57ed161b33fa1f157ce663ed85e69640c25
---

diff --git a/folly/futures/Future.cpp b/folly/futures/Future.cpp
index dc419433..33eb9b94 100644
--- a/folly/futures/Future.cpp
+++ b/folly/futures/Future.cpp
@@ -41,8 +41,13 @@ Future<Unit> sleep(Duration dur, Timekeeper* tk) {
   std::shared_ptr<Timekeeper> tks;
   if (LIKELY(!tk)) {
     tks = folly::detail::getTimekeeperSingleton();
-    tk = DCHECK_NOTNULL(tks.get());
+    tk = tks.get();
   }
+
+  if (UNLIKELY(!tk)) {
+    return makeFuture<Unit>(NoTimekeeper());
+  }
+
   return tk->after(dur);
 }
 
diff --git a/folly/futures/FutureException.h b/folly/futures/FutureException.h
index 5d265b75..2b95f9a4 100644
--- a/folly/futures/FutureException.h
+++ b/folly/futures/FutureException.h
@@ -83,9 +83,15 @@ class FOLLY_EXPORT PredicateDoesNotObtain : public FutureException {
 
 [[noreturn]] void throwPredicateDoesNotObtain();
 
-struct FOLLY_EXPORT NoFutureInSplitter : FutureException {
+class FOLLY_EXPORT NoFutureInSplitter : FutureException {
+ public:
   NoFutureInSplitter() : FutureException("No Future in this FutureSplitter") {}
 };
 
 [[noreturn]] void throwNoFutureInSplitter();
+
+class FOLLY_EXPORT NoTimekeeper : public FutureException {
+ public:
+  NoTimekeeper() : FutureException("No timekeeper available") {}
+};
 }
diff --git a/folly/futures/test/TimekeeperTest.cpp b/folly/futures/test/TimekeeperTest.cpp
index bc3a8aed..4567b93c 100644
--- a/folly/futures/test/TimekeeperTest.cpp
+++ b/folly/futures/test/TimekeeperTest.cpp
@@ -15,6 +15,8 @@
  */
 
 #include <folly/futures/Timekeeper.h>
+#include <folly/Singleton.h>
+#include <folly/futures/ThreadWheelTimekeeper.h>
 #include <folly/portability/GTest.h>
 
 using namespace folly;
@@ -78,6 +80,14 @@ TEST(Timekeeper, futureSleep) {
   EXPECT_GE(now() - t1, one_ms);
 }
 
+TEST(Timekeeper, futureSleepHandlesNullTimekeeperSingleton) {
+  Singleton<ThreadWheelTimekeeper>::make_mock([] { return nullptr; });
+  SCOPE_EXIT {
+    Singleton<ThreadWheelTimekeeper>::make_mock();
+  };
+  EXPECT_THROW(futures::sleep(one_ms).get(), NoTimekeeper);
+}
+
 TEST(Timekeeper, futureDelayed) {
   auto t1 = now();
   auto dur = makeFuture()