Skip to content

Commit

Permalink
Fix crash with async tree deletions if the executor has not yet initi…
Browse files Browse the repository at this point in the history
…alized.

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 #7527.

RELNOTES: None.
PiperOrigin-RevId: 268512558
  • Loading branch information
jmmv authored and copybara-github committed Sep 11, 2019
1 parent 2d0046a commit 9ce145a
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
43 changes: 43 additions & 0 deletions src/test/shell/integration/sandboxing_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 9ce145a

Please sign in to comment.