From a5af72a1692aa023b65e7a0fb3506d76c75fc2c2 Mon Sep 17 00:00:00 2001
From: John Knox <jknox@fb.com>
Date: Mon, 3 Sep 2018 11:10:52 -0700
Subject: [PATCH] Reject null event base arguments

Summary:
If you use a null eventbase, folly implicitly decides which one to run the futures in, for example
the timekeeper thread.
The only component that passes in event bases is sonar.cpp.

Reviewed By: priteshrnandgaonkar

Differential Revision: D9539443

fbshipit-source-id: 7fd7289257c84b039a7ac00b14f78bb271262480
---
 xplat/Sonar/SonarWebSocketImpl.cpp            |  6 +-
 .../SonarWebSocketImplTerminationTests.cpp    | 90 +++++++++++++++++++
 2 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp

diff --git a/xplat/Sonar/SonarWebSocketImpl.cpp b/xplat/Sonar/SonarWebSocketImpl.cpp
index 75468c801..e4e0e9fba 100644
--- a/xplat/Sonar/SonarWebSocketImpl.cpp
+++ b/xplat/Sonar/SonarWebSocketImpl.cpp
@@ -21,6 +21,7 @@
 #include <iostream>
 #include <thread>
 #include <folly/io/async/AsyncSocketException.h>
+#include <stdexcept>
 #include "CertificateUtils.h"
 
 #ifdef __ANDROID__
@@ -96,7 +97,10 @@ class Responder : public rsocket::RSocketResponder {
 };
 
 SonarWebSocketImpl::SonarWebSocketImpl(SonarInitConfig config, std::shared_ptr<SonarState> state)
-    : deviceData_(config.deviceData), sonarState_(state), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker) {}
+    : deviceData_(config.deviceData), sonarState_(state), sonarEventBase_(config.callbackWorker), connectionEventBase_(config.connectionWorker) {
+      CHECK_THROW(config.callbackWorker, std::invalid_argument);
+      CHECK_THROW(config.connectionWorker, std::invalid_argument);
+    }
 
 SonarWebSocketImpl::~SonarWebSocketImpl() {
   stop();
diff --git a/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp b/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp
new file mode 100644
index 000000000..11f5cd435
--- /dev/null
+++ b/xplat/SonarTests/SonarWebSocketImplTerminationTests.cpp
@@ -0,0 +1,90 @@
+/*
+ *  Copyright (c) 2018-present, Facebook, Inc.
+ *
+ *  This source code is licensed under the MIT license found in the LICENSE
+ *  file in the root directory of this source tree.
+ *
+ */
+
+#include <Sonar/SonarWebSocketImpl.h>
+
+#include <gtest/gtest.h>
+
+namespace facebook {
+namespace sonar {
+namespace test {
+
+using folly::EventBase;
+
+class SonarWebSocketImplTerminationTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    // Folly singletons must be registered before they are used.
+    // Without this, test fails in phabricator.
+    folly::SingletonVault::singleton()->registrationComplete();
+  }
+};
+
+TEST_F(SonarWebSocketImplTerminationTest, testNullEventBaseGetsRejected) {
+  try {
+    auto instance = std::make_shared<SonarWebSocketImpl>(SonarInitConfig {
+      DeviceData {},
+      nullptr,
+      new EventBase()
+    }, std::make_shared<SonarState>());
+    FAIL();
+  } catch (std::invalid_argument& e) {
+    // Pass test
+  }
+  try {
+    auto instance = std::make_shared<SonarWebSocketImpl>(SonarInitConfig {
+      DeviceData {},
+      new EventBase(),
+      nullptr
+    }, std::make_shared<SonarState>());
+    FAIL();
+  } catch (std::invalid_argument& e) {
+    // Pass test
+  }
+}
+
+TEST_F(SonarWebSocketImplTerminationTest, testNonStartedEventBaseDoesntHang) {
+  auto config = SonarInitConfig {
+    DeviceData {},
+    new EventBase(),
+    new EventBase()
+  };
+  auto state = std::make_shared<SonarState>();
+  auto instance = std::make_shared<SonarWebSocketImpl>(config, state);
+  instance->start();
+}
+
+TEST_F(SonarWebSocketImplTerminationTest, testStartedEventBaseDoesntHang) {
+  auto sonarEventBase = new EventBase();
+  auto connectionEventBase = new EventBase();
+  auto sonarThread = std::thread([sonarEventBase](){
+    sonarEventBase->loopForever();
+  });
+  auto connectionThread = std::thread([connectionEventBase](){
+    connectionEventBase->loopForever();
+  });
+  auto config = SonarInitConfig {
+    DeviceData {},
+    sonarEventBase,
+    connectionEventBase
+  };
+  auto state = std::make_shared<SonarState>();
+  auto instance = std::make_shared<SonarWebSocketImpl>(config, state);
+
+  instance->start();
+
+  sonarEventBase->terminateLoopSoon();
+  connectionEventBase->terminateLoopSoon();
+
+  sonarThread.join();
+  connectionThread.join();
+}
+
+} // namespace test
+} // namespace sonar
+} // namespace facebook