From 9ce145a34d5dd246ce370ce3c3c0994d3155324d Mon Sep 17 00:00:00 2001
From: jmmv <jmmv@google.com>
Date: Wed, 11 Sep 2019 12:08:06 -0700
Subject: [PATCH] Fix crash with async tree deletions if the executor has not
 yet initialized.

By the time afterCommand runs, we don't know if executorInit has run so we
cannot make any assertions about the state modified by the latter.  The code
contained one such assertion that relied on the asynchronous tree deleter
being initialized, which caused a crash in some circumstances.

While doing this, add a test to validate this in the general case for the
sandbox module given that this is not the first time we spot a similar issue
in it.

Follow up to https://github.com/bazelbuild/bazel/issues/7527.

RELNOTES: None.
PiperOrigin-RevId: 268512558
---
 .../build/lib/sandbox/SandboxModule.java      |  2 +-
 src/test/shell/integration/sandboxing_test.sh | 43 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
index efed9c1893cb85..5f01a5ab4426a4 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxModule.java
@@ -509,7 +509,7 @@ public void afterCommand() {
 
     SandboxOptions options = env.getOptions().getOptions(SandboxOptions.class);
     int asyncTreeDeleteThreads = options != null ? options.asyncTreeDeleteIdleThreads : 0;
-    if (asyncTreeDeleteThreads > 0) {
+    if (treeDeleter != null && asyncTreeDeleteThreads > 0) {
       // If asynchronous deletions were requested, they may still be ongoing so let them be: trying
       // to delete the base tree synchronously could fail as we can race with those other deletions,
       // and scheduling an asynchronous deletion could race with future builds.
diff --git a/src/test/shell/integration/sandboxing_test.sh b/src/test/shell/integration/sandboxing_test.sh
index e7f88c34202690..683fefe640bd8a 100755
--- a/src/test/shell/integration/sandboxing_test.sh
+++ b/src/test/shell/integration/sandboxing_test.sh
@@ -75,6 +75,49 @@ function test_sandbox_base_wiped_only_on_startup_with_async_deletions() {
     --experimental_sandbox_async_tree_delete_idle_threads=HOST_CPUS
 }
 
+function do_succeed_when_executor_not_initialized_test() {
+  local extra_args=( "${@}" )
+
+  mkdir pkg
+  mkfifo pkg/BUILD
+
+  bazel build --spawn_strategy=sandboxed --nobuild "${@}" //pkg:all \
+    >"${TEST_log}" 2>&1 &
+  local pid="${!}"
+
+  echo "Waiting for Blaze to finish initializing all modules"
+  while ! grep "currently loading: pkg" "${TEST_log}"; do
+    sleep 1
+  done
+
+  echo "Interrupting Blaze before it gets to init the executor"
+  kill "${pid}"
+
+  echo "And now giving Blaze a chance to finalize all modules"
+  echo "unblock fifo" >pkg/BUILD
+  wait "${pid}" || true
+
+  expect_log "Build did NOT complete successfully"
+  # Disallow some common messages we might see during a crash.
+  expect_not_log "Internal error"
+  expect_not_log "stack trace"
+  expect_not_log "NullPointerException"
+}
+
+function test_succeed_when_executor_not_initialized_with_defaults() {
+  # Pass a no-op flag to the test to workaround a bug in macOS's default
+  # and ancient bash version which causes it to error out on an empty
+  # argument list when $@ is consumed and set -u is enabled.
+  local noop=( --nobuild )
+
+  do_succeed_when_executor_not_initialized_test "${noop[@]}"
+}
+
+function test_succeed_when_executor_not_initialized_with_async_deletions() {
+  do_succeed_when_executor_not_initialized_test \
+    --experimental_sandbox_async_tree_delete_idle_threads=auto
+}
+
 function test_sandbox_base_can_be_rm_rfed() {
   mkdir pkg
   cat >pkg/BUILD <<EOF