[kudu] 02/02: mini-cluster: disallow restarting daemons from other threads

[aw...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

awong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git

commit 0cded868fd3d8ff088cd290dccdd9d90fdc0817c
Author: Andrew Wong <aw...@apache.org>
AuthorDate: Fri Jan 17 18:46:58 2020 -0800

    mini-cluster: disallow restarting daemons from other threads
    
    If we restart an external daemon from a separate thread, the daemon can be
    killed silently and without warning when the thread is reaped.
    
    For instance, the following would fail without logging any information about
    the tserver dying:
    
    TEST_F(ExternalMiniClusterITestBase, TestRestartFromThread) {
      ExternalMiniClusterOptions opts;
      opts.num_tablet_servers = 1;
      NO_FATALS(StartClusterWithOpts(std::move(opts)));
      thread t([&] {
        auto* ts = cluster_->tablet_server(0);
        ts->Shutdown();
        return ts->Restart();
      });
      t.join();
      SleepFor(MonoDelta::FromSeconds(1));
      ASSERT_TRUE(cluster_->tablet_server(0)->IsProcessAlive());
    }
    
    I didn't add a death test, since death tests themselves don't work well in
    multithreaded contexts.
    
    Change-Id: I184a01be3e1ac7f60a8b3aedab176dc9138033e0
    Reviewed-on: http://gerrit.cloudera.org:8080/15069
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/mini-cluster/external_mini_cluster.cc | 9 +++++++--
 src/kudu/mini-cluster/external_mini_cluster.h  | 7 +++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/src/kudu/mini-cluster/external_mini_cluster.cc b/src/kudu/mini-cluster/external_mini_cluster.cc
index 0120dc9..54eae8f 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.cc
+++ b/src/kudu/mini-cluster/external_mini_cluster.cc
@@ -25,11 +25,11 @@
 #include <iterator>
 #include <memory>
 #include <string>
+#include <thread>
 #include <unordered_set>
 #include <utility>
 
 #include <gflags/gflags.h>
-#include <gflags/gflags_declare.h>
 #include <gtest/gtest.h>
 
 #include "kudu/client/client.h"
@@ -939,7 +939,8 @@ string ExternalMiniCluster::UuidForTS(int ts_idx) const {
 //------------------------------------------------------------
 
 ExternalDaemon::ExternalDaemon(ExternalDaemonOptions opts)
-    : opts_(std::move(opts)) {
+    : opts_(std::move(opts)),
+      parent_tid_(std::this_thread::get_id()) {
   CHECK(rpc_bind_address().Initialized());
 }
 
@@ -968,6 +969,10 @@ Status ExternalDaemon::EnableKerberos(MiniKdc* kdc, const string& bind_host) {
 
 Status ExternalDaemon::StartProcess(const vector<string>& user_flags) {
   CHECK(!process_);
+  const auto this_tid = std::this_thread::get_id();
+  CHECK_EQ(parent_tid_, this_tid)
+    << "Process being started from thread " << this_tid << " which is different"
+    << " from the instantiating thread " << parent_tid_;
 
   vector<string> argv;
 
diff --git a/src/kudu/mini-cluster/external_mini_cluster.h b/src/kudu/mini-cluster/external_mini_cluster.h
index 428ec3a..5b7b81d 100644
--- a/src/kudu/mini-cluster/external_mini_cluster.h
+++ b/src/kudu/mini-cluster/external_mini_cluster.h
@@ -25,6 +25,8 @@
 #include <memory>
 #include <ostream>
 #include <string>
+#include <thread>
+#include <utility>
 #include <vector>
 
 #include <boost/optional/optional.hpp>
@@ -610,6 +612,11 @@ class ExternalDaemon : public RefCountedThreadSafe<ExternalDaemon> {
   HostPort bound_rpc_;
   HostPort bound_http_;
 
+  // ID of the thread that is spawning the child processes. This should not
+  // change across restarts of the daemon, as forking from different threads
+  // may yield behavior like daemons being killed when the new thread exits.
+  const std::thread::id parent_tid_;
+
   DISALLOW_COPY_AND_ASSIGN(ExternalDaemon);
 };