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 dc419433ff84c9f00106e532a23e2fe6fd2915b1..33eb9b94efee58de01e3e466a8ecff30227cb12e 100644 (file)
--- 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 5d265b75b76747b66a334ac7117837aff4286563..2b95f9a437b1d074123f66b210778d384c241650 100644 (file)
--- 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 bc3a8aed219b94779c6fd64dd69d9b9b9dd5d2cd..4567b93c3f1b70558e29d4e477f1af7e252634ec 100644 (file)
--- 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()