Use 'exec*()' to perform client and server death tests

Before this commit we tried to perform the death part of the client or
server death tests in the forked child process created from
'ASSERT_DEATH()'. This has "worked" but investigating ongoing flaky
tests suggests this approach is insufficient. This commit does less in
the forked child by immediately doing an 'exec*()'.

test/BUILD.bazel

@@ -1,4 +1,26 @@
-load("@rules_cc//cc:defs.bzl", "cc_test")
+load("@rules_cc//cc:defs.bzl", "cc_binary", "cc_test")
+
+cc_binary(
+    name = "death-client",
+    srcs = [
+        "death-client.cc",
+    ],
+    deps = [
+        "//:grpc",
+        "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc",
+    ],
+)
+
+cc_binary(
+    name = "death-server",
+    srcs = [
+        "death-server.cc",
+    ],
+    deps = [
+        "//:grpc",
+        "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc",
+    ],
+)
 
 cc_test(
     name = "grpc",
@@ -13,6 +35,7 @@ cc_test(
         "greeter-server.cc",
         "helloworld.eventuals.cc",
         "helloworld.eventuals.h",
+        "main.cc",
         "multiple-hosts.cc",
         "server-death-test.cc",
         "server-unavailable.cc",
@@ -21,13 +44,18 @@ cc_test(
         "unary.cc",
         "unimplemented.cc",
     ],
+    data = [
+        ":death-client",
+        ":death-server",
+    ],
     # NOTE: need to add 'linkstatic = True' in order to get this to
     # link until https://github.com/grpc/grpc/issues/13856 gets
     # resolved.
     linkstatic = True,
     deps = [
         "//:grpc",
-        "@com_github_google_googletest//:gtest_main",
+        "@bazel_tools//tools/cpp/runfiles",
+        "@com_github_google_googletest//:gtest",
         "@com_github_grpc_grpc//examples/protos:helloworld_cc_grpc",
         "@com_github_grpc_grpc//examples/protos:keyvaluestore",
     ],

test/client-death-test.cc

@@ -1,9 +1,6 @@
-#include "eventuals/eventual.h"
-#include "eventuals/grpc/client.h"
 #include "eventuals/grpc/server.h"
 #include "eventuals/head.h"
 #include "eventuals/let.h"
-#include "eventuals/loop.h"
 #include "eventuals/terminal.h"
 #include "eventuals/then.h"
 #include "examples/protos/helloworld.grpc.pb.h"
@@ -14,17 +11,11 @@ using helloworld::Greeter;
 using helloworld::HelloReply;
 using helloworld::HelloRequest;
 
-using stout::Borrowable;
-
-using eventuals::Eventual;
 using eventuals::Head;
 using eventuals::Let;
-using eventuals::Loop;
 using eventuals::Terminate;
 using eventuals::Then;
 
-using eventuals::grpc::Client;
-using eventuals::grpc::CompletionPool;
 using eventuals::grpc::Server;
 using eventuals::grpc::ServerBuilder;
 
@@ -42,50 +33,49 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) {
   ASSERT_NE(-1, pipe(pipes.fork));
   ASSERT_NE(-1, pipe(pipes.port));
 
-  auto wait_for_fork = [&]() {
+  auto WaitForFork = [&]() {
     int _;
     CHECK_GT(read(pipes.fork[0], &_, sizeof(int)), 0);
   };
 
-  auto notify_forked = [&]() {
-    int _ = 1;
-    CHECK_GT(write(pipes.fork[1], &_, sizeof(int)), 0);
-  };
-
-  auto wait_for_port = [&]() {
-    int port;
-    CHECK_GT(read(pipes.port[0], &port, sizeof(int)), 0);
-    return port;
-  };
-
-  auto send_port = [&](int port) {
+  auto SendPort = [&](int port) {
     CHECK_GT(write(pipes.port[1], &port, sizeof(port)), 0);
   };
 
-  auto client = [&]() {
-    notify_forked();
-
-    int port = wait_for_port();
-
-    Borrowable<CompletionPool> pool;
-
-    Client client(
-        "0.0.0.0:" + stringify(port),
-        grpc::InsecureChannelCredentials(),
-        pool.Borrow());
-
-    auto call = [&]() {
-      return client.Call<Greeter, HelloRequest, HelloReply>("SayHello")
-          | Then([](auto&& call) {
-               exit(1);
-             });
-    };
-
-    *call();
-  };
-
   std::thread thread([&]() {
-    ASSERT_DEATH(client(), "");
+    std::string path = GetRunfilePathFor("death-client").string();
+
+    std::string pipe_fork = std::to_string(pipes.fork[1]);
+    std::string pipe_port = std::to_string(pipes.port[0]);
+
+    // NOTE: doing a 'fork()' when a parent has multiple threads is
+    // wrought with potential issues because the child only gets a
+    // single one of those threads (the one that called 'fork()'). In
+    // our case we _need_ a thread so that we can simultaneously run
+    // both the client and the server, but we ensure here that there
+    // is only the single extra thread.
+    ASSERT_EQ(GetThreadCount(), 2);
+
+    ASSERT_DEATH(
+        [&]() {
+          // Conventional wisdom is to do the least amount possible
+          // after a 'fork()', ideally just an 'exec*()', and that's
+          // what we're doing because when we tried to do more the
+          // tests were flaky likely do to some library that doesn't
+          // properly work after doing a 'fork()'.
+          execl(
+              path.c_str(),
+              path.c_str(),
+              pipe_fork.c_str(),
+              pipe_port.c_str(),
+              NULL);
+
+          // NOTE: if 'execve()' failed then this lamdba will return
+          // and gtest will see consider this "death" to be a failure.
+        }(),
+        // NOTE: to avoid false positives we check for a death with
+        // the string 'connected' printed to stderr.
+        "connected");
   });
 
   // NOTE: we detach the thread so that there isn't a race with the
@@ -97,7 +87,9 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) {
   // test which might have been destructed before it destructs.
   thread.detach();
 
-  wait_for_fork();
+  // NOTE: need to wait to call into gRPC till _after_ we've forked
+  // (see comment at top of test for more details).
+  WaitForFork();
 
   ServerBuilder builder;
 
@@ -128,7 +120,9 @@ TEST_F(EventualsGrpcTest, ClientDeathTest) {
 
   k.Start();
 
-  send_port(port);
+  // NOTE: sending this _after_ we start the eventual so that we're
+  // ready to accept clients!
+  SendPort(port);
 
   EXPECT_TRUE(cancelled.get());
 

test/death-client.cc

@@ -0,0 +1,62 @@
+#include "eventuals/grpc/client.h"
+#include "eventuals/terminal.h"
+#include "eventuals/then.h"
+#include "examples/protos/helloworld.grpc.pb.h"
+
+using helloworld::Greeter;
+using helloworld::HelloReply;
+using helloworld::HelloRequest;
+
+using stout::Borrowable;
+
+using eventuals::Eventual;
+using eventuals::Terminate;
+using eventuals::Then;
+
+using eventuals::grpc::Client;
+using eventuals::grpc::CompletionPool;
+
+int main(int argc, char** argv) {
+  CHECK(argc == 3)
+      << "expecting 'pipe_fork' and 'pipe_port' to be passed as arguments";
+
+  int pipe_fork = atoi(argv[1]);
+  int pipe_port = atoi(argv[2]);
+
+  auto NotifyForked = [&]() {
+    int one = 1;
+    CHECK_GT(write(pipe_fork, &one, sizeof(int)), 0);
+  };
+
+  auto WaitForPort = [&]() {
+    int port;
+    CHECK_GT(read(pipe_port, &port, sizeof(int)), 0);
+    return port;
+  };
+
+  NotifyForked();
+
+  int port = WaitForPort();
+
+  Borrowable<CompletionPool> pool;
+
+  Client client(
+      "0.0.0.0:" + std::to_string(port),
+      grpc::InsecureChannelCredentials(),
+      pool.Borrow());
+
+  auto call = [&]() {
+    return client.Call<Greeter, HelloRequest, HelloReply>("SayHello")
+        | Then([](auto&& call) {
+             // NOTE: to avoid false positives with, for example, one
+             // of the 'CHECK()'s failing above, the 'ClientDeathTest'
+             // expects the string 'connected' to be written to stderr.
+             std::cerr << "connected" << std::endl;
+             exit(1);
+           });
+  };
+
+  *call();
+
+  return 0;
+}

test/death-server.cc

@@ -0,0 +1,65 @@
+#include "eventuals/grpc/server.h"
+#include "eventuals/head.h"
+#include "eventuals/terminal.h"
+#include "eventuals/then.h"
+#include "examples/protos/helloworld.grpc.pb.h"
+
+using helloworld::Greeter;
+using helloworld::HelloReply;
+using helloworld::HelloRequest;
+
+using eventuals::Head;
+using eventuals::Terminate;
+using eventuals::Then;
+
+using eventuals::grpc::Server;
+using eventuals::grpc::ServerBuilder;
+
+int main(int argc, char** argv) {
+  CHECK(argc == 2) << "expecting 'pipe' to be passed as an argument";
+
+  int pipe = atoi(argv[1]);
+
+  auto SendPort = [&](int port) {
+    CHECK_GT(write(pipe, &port, sizeof(port)), 0);
+  };
+
+  ServerBuilder builder;
+
+  int port = 0;
+
+  builder.AddListeningPort(
+      "0.0.0.0:0",
+      grpc::InsecureServerCredentials(),
+      &port);
+
+  auto build = builder.BuildAndStart();
+
+  CHECK(build.status.ok());
+
+  auto server = std::move(build.server);
+
+  CHECK(server);
+
+  auto serve = [&]() {
+    return server->Accept<Greeter, HelloRequest, HelloReply>("SayHello")
+        | Head()
+        | Then([](auto&& call) {
+             // NOTE: to avoid false positives with, for example, one
+             // of the 'CHECK()'s failing above, the 'ServerDeathTest'
+             // expects the string 'accepted' to be written to stderr.
+             std::cerr << "accepted" << std::endl;
+             exit(1);
+           });
+  };
+
+  auto [future, k] = Terminate(serve());
+
+  k.Start();
+
+  // NOTE: sending this _after_ we start the eventual so that we're
+  // ready to accept clients!
+  SendPort(port);
+
+  future.get();
+}

test/main.cc

@@ -0,0 +1,50 @@
+#include "gtest/gtest.h"
+#include "test/test.h"
+#include "tools/cpp/runfiles/runfiles.h"
+
+////////////////////////////////////////////////////////////////////////
+
+// NOTE: using a raw pointer here as per Google C++ Style Guide
+// because 'bazel::tools::cpp::runfiles::Runfiles' is not trivially
+// destructible.
+static bazel::tools::cpp::runfiles::Runfiles* runfiles = nullptr;
+
+////////////////////////////////////////////////////////////////////////
+
+// Declared in test.h.
+std::filesystem::path GetRunfilePathFor(const std::filesystem::path& runfile) {
+  std::string path = CHECK_NOTNULL(runfiles)->Rlocation(runfile);
+
+  CHECK(!path.empty()) << "runfile " << runfile << " does not exist";
+
+  return path;
+}
+
+////////////////////////////////////////////////////////////////////////
+
+int main(int argc, char** argv) {
+  std::string error;
+
+  // NOTE: using 'Create()' instead of 'CreateForTest()' so that we
+  // can support both running the test via 'bazel test' or explicitly
+  // (i.e., ./path/to/test --gtest_...).
+  runfiles = bazel::tools::cpp::runfiles::Runfiles::Create(
+      argv[0],
+      std::string(),
+      std::filesystem::absolute(argv[0]).parent_path().string(),
+      &error);
+
+  if (runfiles == nullptr) {
+    std::cerr
+        << "Failed to construct 'Runfiles' necessary for getting "
+        << "paths to assets needed in order to run tests: "
+        << error << std::endl;
+    return -1;
+  }
+
+  ::testing::InitGoogleTest(&argc, argv);
+
+  return RUN_ALL_TESTS();
+}
+
+////////////////////////////////////////////////////////////////////////