client: actually terminate the tunnel-encap Envoy subprocess

source/client/process_bootstrap.cc

@@ -445,20 +445,27 @@ absl::Status
 EncapsulationSubProcessRunner::RunWithSubprocess(std::function<void()> nighthawk_fn,
                                                  std::function<void(sem_t&)> envoy_fn) {
 
-  pid_t pid_ = fork();
+  pid_ = fork();
   if (pid_ == -1) {
     return absl::InternalError("fork failed");
   }
   if (pid_ == 0) {
     envoy_fn(*nighthawk_control_sem_);
-    exit(0);
+    // Use _exit() rather than exit(): this child is a fork of a potentially multithreaded
+    // parent (e.g. the gRPC service), where only the forking thread survives. exit() would
+    // run atexit/static-destructor/sanitizer hooks that can deadlock on locks owned by
+    // threads that no longer exist, wedging the child - and, now that the parent reaps the
+    // child via waitpid(), wedging the parent's shutdown along with it.
+    _exit(0);
   } else {
-    // wait for envoy to start and signal nighthawk to start
+    // Wait for envoy to start and signal nighthawk to start. Note that nighthawk_fn only
+    // starts execution and returns; the encap subprocess must outlive the load test, so it
+    // is NOT terminated here. Termination happens via TerminateEncapSubProcess(), which
+    // ProcessImpl invokes on shutdown and on execution cancellation (and the destructor
+    // invokes as a last resort).
     sem_wait(nighthawk_control_sem_);
     // start nighthawk
     nighthawk_fn();
-    // signal envoy to shutdown
-    return TerminateEncapSubProcess();
   }
   return absl::OkStatus();
 }

source/client/process_bootstrap.h

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <semaphore.h>
+#include <unistd.h>
 
 #include <functional>
 #include <vector>
@@ -106,7 +107,10 @@ class EncapsulationSubProcessRunner {
   absl::Status Run() { return RunWithSubprocess(nighthawk_runner_, encap_envoy_runner_); }
 
   /**
-   * Sends a SIGTERM to Encap Envoy subprocess and blocks till exit
+   * Sends a SIGTERM to the Encap Envoy subprocess and reaps it. Waits up to ~15 seconds
+   * for a graceful exit, then escalates to SIGKILL: an unbounded wait here could wedge the
+   * caller (for example a gRPC service handler running ProcessImpl::shutdown()) if the
+   * subprocess never exits.
    *
    **/
   absl::Status TerminateEncapSubProcess() {
@@ -119,16 +123,31 @@ class EncapsulationSubProcessRunner {
       return absl::InternalError("Failed to kill encapsulation subprocess");
     }
 
-    int status;
-    waitpid(pid_, &status, 0);
+    int status = 0;
+    pid_t reaped = 0;
+    for (int i = 0; i < 150 && reaped == 0; ++i) {
+      reaped = waitpid(pid_, &status, WNOHANG);
+      if (reaped == 0) {
+        // Waiting on a real OS process; there is no injectable time source here.
+        usleep(100000); // 100ms per attempt, ~15s in total. NO_CHECK_FORMAT(real_time)
+      }
+    }
+    if (reaped == 0) {
+      kill(pid_, SIGKILL);
+      reaped = waitpid(pid_, &status, 0);
+    }
     pid_ = -1;
 
+    if (reaped < 0) {
+      // Nothing to reap (e.g. the child was already reaped elsewhere): not a crash.
+      return absl::OkStatus();
+    }
     if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
       // Child process did not crash.
       return absl::OkStatus();
     }
 
-    // Child process crashed.
+    // Child process crashed or had to be hard-killed.
     return absl::InternalError(absl::StrCat("Envoy crashed with code: ", status));
   }