From e7151f971e1a633917ded8f7a12048b3273fd35c Mon Sep 17 00:00:00 2001
From: Colin Cross <ccross@android.com>
Date: Tue, 28 Nov 2023 14:18:12 -0800
Subject: [PATCH] Move startRBE error handling to the main goroutine

When called from the startRBE goroutine, the panic ctx.Fatalf triggered
by is unhandled and causes the process to exit without cleaning up the
terminal.  Move most of the quick checks to a checkRBERequirements
function called on the main goroutine so they can be reported
immediately before running Soong and Kati, and recover any panics
from startRBE and report them after waiting on rbeCh.

Bug: 293501187
Test: gcertdestroy && m nothing
Change-Id: I62c84c93ab0a7f0e4f2ab2cc64b22e2070dd6e4c
---
 ui/build/build.go | 12 +++++++++++-
 ui/build/rbe.go   | 13 ++++++++-----
 2 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/ui/build/build.go b/ui/build/build.go
index 9d5c330e1..54a4c27e5 100644
--- a/ui/build/build.go
+++ b/ui/build/build.go
@@ -283,11 +283,16 @@ func Build(ctx Context, config Config) {
 	}
 
 	rbeCh := make(chan bool)
+	var rbePanic any
 	if config.StartRBE() {
 		cleanupRBELogsDir(ctx, config)
+		checkRBERequirements(ctx, config)
 		go func() {
+			defer func() {
+				rbePanic = recover()
+				close(rbeCh)
+			}()
 			startRBE(ctx, config)
-			close(rbeCh)
 		}()
 		defer DumpRBEMetrics(ctx, config, filepath.Join(config.LogsDir(), "rbe_metrics.pb"))
 	} else {
@@ -345,6 +350,11 @@ func Build(ctx Context, config Config) {
 	}
 
 	<-rbeCh
+	if rbePanic != nil {
+		// If there was a ctx.Fatal in startRBE, rethrow it.
+		panic(rbePanic)
+	}
+
 	if what&RunNinja != 0 {
 		if what&RunKati != 0 {
 			installCleanIfNecessary(ctx, config)
diff --git a/ui/build/rbe.go b/ui/build/rbe.go
index 19a54dfc2..fa04207e0 100644
--- a/ui/build/rbe.go
+++ b/ui/build/rbe.go
@@ -95,14 +95,10 @@ func cleanupRBELogsDir(ctx Context, config Config) {
 	}
 }
 
-func startRBE(ctx Context, config Config) {
+func checkRBERequirements(ctx Context, config Config) {
 	if !config.GoogleProdCredsExist() && prodCredsAuthType(config) {
 		ctx.Fatalf("Unable to start RBE reproxy\nFAILED: Missing LOAS credentials.")
 	}
-	ctx.BeginTrace(metrics.RunSetupTool, "rbe_bootstrap")
-	defer ctx.EndTrace()
-
-	ctx.Status.Status("Starting rbe...")
 
 	if u := ulimitOrFatal(ctx, config, "-u"); u < rbeLeastNProcs {
 		ctx.Fatalf("max user processes is insufficient: %d; want >= %d.\n", u, rbeLeastNProcs)
@@ -115,6 +111,13 @@ func startRBE(ctx Context, config Config) {
 			ctx.Fatalf("Unable to create logs dir (%v) for RBE: %v", config.rbeProxyLogsDir, err)
 		}
 	}
+}
+
+func startRBE(ctx Context, config Config) {
+	ctx.BeginTrace(metrics.RunSetupTool, "rbe_bootstrap")
+	defer ctx.EndTrace()
+
+	ctx.Status.Status("Starting rbe...")
 
 	cmd := Command(ctx, config, "startRBE bootstrap", rbeCommand(ctx, config, bootstrapCmd))