The current implementation of `Sandbox::Terminate` results in timeout's being reported to coroner in cases where a Restart or Terminate with graceful exit is requested.

This change requests an exit from the sandboxee and then awaits the result either with a timeout of 1 second (the grace period) or else with infinite duration - which would then report the timeout again.

PiperOrigin-RevId: 589128986
Change-Id: Icc948b37f13f46af907fd1eab649cabb5ed50b25

2 files changed, 21 insertions, 19 deletions

diff --git a/sandboxed_api/rpcchannel.cc b/sandboxed_api/rpcchannel.cc
index 7eeb447..7e76b05 100644
--- a/sandboxed_api/rpcchannel.cc
+++ b/sandboxed_api/rpcchannel.cc
@@ -140,16 +140,6 @@ absl::Status RPCChannel::Exit() {
   // Try the RPC exit sequence. But, the only thing that matters as a success
   // indicator is whether the Comms channel had been closed
   comms_->SendTLV(comms::kMsgExit, 0, nullptr);
-  bool unused;
-  comms_->RecvBool(&unused);
-
-  if (!comms_->IsTerminated()) {
-    LOG(ERROR) << "Comms channel not terminated in Exit()";
-    // TODO(hamacher): Better error code
-    return absl::FailedPreconditionError(
-        "Comms channel not terminated in Exit()");
-  }
-
   return absl::OkStatus();
 }
 
diff --git a/sandboxed_api/sandbox.cc b/sandboxed_api/sandbox.cc
index b134ac9..5f0f9e6 100644
--- a/sandboxed_api/sandbox.cc
+++ b/sandboxed_api/sandbox.cc
@@ -40,6 +40,7 @@
 #include "sandboxed_api/sandbox2/executor.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/policybuilder.h"
+#include "sandboxed_api/sandbox2/result.h"
 #include "sandboxed_api/sandbox2/sandbox2.h"
 #include "sandboxed_api/sandbox2/util/bpf_helper.h"
 #include "sandboxed_api/util/fileops.h"
@@ -105,20 +106,31 @@ void Sandbox::Terminate(bool attempt_graceful_exit) {
     return;
   }
 
+  absl::StatusOr<sandbox2::Result> result;
   if (attempt_graceful_exit) {
-    // Gracefully ask it to exit (with 1 second limit) first, then kill it.
-    Exit();
-  } else {
-    // Kill it straight away
+    if (absl::Status requested_exit = rpc_channel_->Exit();
+        !requested_exit.ok()) {
+      LOG(WARNING)
+          << "rpc_channel->Exit() failed, calling AwaitResultWithTimeout(1) "
+          << requested_exit;
+    }
+    result = s2_->AwaitResultWithTimeout(absl::Seconds(1));
+    if (!result.ok()) {
+      LOG(WARNING) << "s2_->AwaitResultWithTimeout failed, status: "
+                   << result.status() << " Killing PID: " << pid();
+    }
+  }
+
+  if (!attempt_graceful_exit || !result.ok()) {
     s2_->Kill();
+    result = s2_->AwaitResult();
   }
 
-  const auto& result = AwaitResult();
-  if (result.final_status() == sandbox2::Result::OK &&
-      result.reason_code() == 0) {
-    VLOG(2) << "Sandbox2 finished with: " << result.ToString();
+  if (result->final_status() == sandbox2::Result::OK &&
+      result->reason_code() == 0) {
+    VLOG(2) << "Sandbox2 finished with: " << result->ToString();
   } else {
-    LOG(WARNING) << "Sandbox2 finished with: " << result.ToString();
+    LOG(WARNING) << "Sandbox2 finished with: " << result->ToString();
   }
 }