Handle crashes in a more consistent way.
Unifies the crash handling of exceptions in async threads and exceptions in Skyframe. Both now go through BugReport#handleCrash and BlazeModule#blazeShutdownOnCrash.
Additionally, OOMs from a Skyframe thread are now reported to BES.
PiperOrigin-RevId: 339110985
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index e8ba5d5..b119cd4 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -210,6 +210,7 @@
"runtime/BlazeCommandResult.java",
],
deps = [
+ "//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/concurrent",
"//src/main/java/com/google/devtools/build/lib/util:crash_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
diff --git a/src/main/java/com/google/devtools/build/lib/bugreport/BUILD b/src/main/java/com/google/devtools/build/lib/bugreport/BUILD
index 776c765..bc8e2aa 100644
--- a/src/main/java/com/google/devtools/build/lib/bugreport/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/bugreport/BUILD
@@ -13,6 +13,7 @@
srcs = glob(["*.java"]),
deps = [
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
+ "//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/util:TestType",
"//src/main/java/com/google/devtools/build/lib/util:crash_failure_details",
"//src/main/java/com/google/devtools/build/lib/util:custom_exit_code_publisher",
@@ -20,7 +21,6 @@
"//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
"//src/main/java/com/google/devtools/build/lib/util:exit_code",
"//src/main/java/com/google/devtools/build/lib/util:logging",
- "//src/main/java/com/google/devtools/build/lib/util/io:out-err",
"//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/bugreport/BugReport.java b/src/main/java/com/google/devtools/build/lib/bugreport/BugReport.java
index 98ea588..bd09ec5 100644
--- a/src/main/java/com/google/devtools/build/lib/bugreport/BugReport.java
+++ b/src/main/java/com/google/devtools/build/lib/bugreport/BugReport.java
@@ -21,15 +21,12 @@
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
-import com.google.devtools.build.lib.util.CrashFailureDetails;
+import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.util.CustomExitCodePublisher;
import com.google.devtools.build.lib.util.CustomFailureDetailPublisher;
import com.google.devtools.build.lib.util.DetailedExitCode;
-import com.google.devtools.build.lib.util.ExitCode;
import com.google.devtools.build.lib.util.LoggingUtil;
import com.google.devtools.build.lib.util.TestType;
-import com.google.devtools.build.lib.util.io.OutErr;
-import java.io.PrintStream;
import java.util.Arrays;
import java.util.List;
import java.util.logging.Level;
@@ -131,102 +128,60 @@
logException(exception, filterArgs(args), values);
}
- private static void logCrash(Throwable throwable, boolean sendBugReport, String... args) {
- logger.atSevere().withCause(throwable).log("Crash");
- if (sendBugReport) {
- BugReport.sendBugReport(throwable, Arrays.asList(args));
- }
- logThrowableToConsole(throwable);
- }
-
- private static void logThrowableToConsole(Throwable throwable) {
- BugReport.printBug(OutErr.SYSTEM_OUT_ERR, throwable, /*oomMessage=*/ null);
- System.err.println("ERROR: " + getProductName() + " crash in async thread:");
- throwable.printStackTrace();
- }
-
/**
- * Print, log, and then cause the current Blaze command to fail with the specified exit code, and
- * then cause the jvm to terminate.
+ * Convenience method equivalent to calling {@code BugReport.handleCrash(Crash.from(throwable),
+ * CrashContext.halt().withArgs(args)}.
*
- * <p>Has no effect if another crash has already been handled by {@link BugReport}.
- */
- public static RuntimeException handleCrashWithoutSendingBugReport(
- Throwable throwable, ExitCode exitCode, String... args) {
- throw handleCrash(
- throwable,
- /*sendBugReport=*/ false,
- DetailedExitCode.of(exitCode, CrashFailureDetails.forThrowable(throwable)),
- args);
- }
-
- /**
- * Print, log, send a bug report, and then cause the current Blaze command to fail with the
- * specified exit code, and then cause the jvm to terminate.
- *
- * <p>Has no effect if another crash has already been handled by {@link BugReport}.
- */
- public static RuntimeException handleCrash(
- Throwable throwable, ExitCode exitCode, String... args) {
- throw handleCrash(
- throwable,
- /*sendBugReport=*/ true,
- DetailedExitCode.of(exitCode, CrashFailureDetails.forThrowable(throwable)),
- args);
- }
-
- /**
- * Print, log, and send a bug report, and then cause the current Blaze command to fail with an
- * exit code inferred from the given {@link Throwable}, and then cause the jvm to terminate.
- *
- * <p>Has no effect if another crash has already been handled by {@link BugReport}.
+ * <p>Halts the JVM and does not return.
*/
public static RuntimeException handleCrash(Throwable throwable, String... args) {
- throw handleCrash(
- throwable,
- /*sendBugReport=*/ true,
- CrashFailureDetails.detailedExitCodeForThrowable(throwable),
- args);
+ handleCrash(Crash.from(throwable), CrashContext.halt().withArgs(args));
+ throw new IllegalStateException("Should have halted", throwable);
}
- private static RuntimeException handleCrash(
- Throwable throwable,
- boolean sendBugReport,
- DetailedExitCode detailedExitCode,
- String... args) {
- int numericExitCode = detailedExitCode.getExitCode().getNumericExitCode();
+ /**
+ * Handles a {@link Crash} according to the given {@link CrashContext}.
+ *
+ * <p>In the case of {@link CrashContext#halt}, the JVM is {@linkplain Runtime#halt halted}.
+ * Otherwise, for {@link CrashContext#keepAlive}, returns {@code null}, in which case the caller
+ * is responsible for shutting down the server.
+ */
+ public static void handleCrash(Crash crash, CrashContext ctx) {
+ int numericExitCode = crash.getDetailedExitCode().getExitCode().getNumericExitCode();
try {
synchronized (LOCK) {
- if (TestType.isInTest()) {
- unprocessedThrowableInTest = throwable;
- }
+ logger.atSevere().withCause(crash.getThrowable()).log("Handling crash with %s", ctx);
+
// Don't try to send a bug report during a crash in a test, it will throw itself.
- if (!TestType.isInTest() || !sendBugReport) {
- logCrash(throwable, sendBugReport, args);
- } else {
- logThrowableToConsole(throwable);
+ if (TestType.isInTest()) {
+ unprocessedThrowableInTest = crash.getThrowable();
+ } else if (ctx.shouldSendBugReport()) {
+ sendBugReport(crash.getThrowable(), ctx.getArgs());
}
- // TODO(b/167592709): remove verbose logging when bug resolved.
- logger.atInfo().log("Finished logging crash, runtime: %s", runtime);
+
+ String crashMsg = constructCrashMessageWithStackTrace(crash.getThrowable(), ctx);
+ ctx.getEventHandler().handle(Event.fatal(crashMsg));
+
try {
if (runtime != null) {
- runtime.cleanUpForCrash(detailedExitCode);
+ runtime.cleanUpForCrash(crash.getDetailedExitCode());
}
- logger.atInfo().log("Finished runtime cleanup");
- CustomExitCodePublisher.maybeWriteExitStatusFile(numericExitCode);
- logger.atInfo().log("Wrote exit status file");
+ // Writing the exit code status file is only necessary if we are halting. Otherwise, the
+ // caller is responsible for an orderly shutdown with the proper exit code.
+ if (ctx.shouldHaltJvm()) {
+ CustomExitCodePublisher.maybeWriteExitStatusFile(numericExitCode);
+ }
CustomFailureDetailPublisher.maybeWriteFailureDetailFile(
- detailedExitCode.getFailureDetail());
- logger.atInfo().log("Wrote failure detail file");
+ crash.getDetailedExitCode().getFailureDetail());
} finally {
- logger.atInfo().log("Entered inner finally block");
- // Avoid shutdown deadlock issues: If an application shutdown hook crashes, it will
- // trigger our Blaze crash handler (this method). Calling System#exit() here, would
- // therefore induce a deadlock. This call would block on the shutdown sequence completing,
- // but the shutdown sequence would in turn be blocked on this thread finishing. Instead,
- // exit fast via halt().
- Runtime.getRuntime().halt(numericExitCode);
- logger.atSevere().log("Failed to halt (inner block)!");
+ if (ctx.shouldHaltJvm()) {
+ // Avoid shutdown deadlock issues: If an application shutdown hook crashes, it will
+ // trigger our Blaze crash handler (this method). Calling System#exit() here, would
+ // therefore induce a deadlock. This call would block on the shutdown sequence
+ // completing, but the shutdown sequence would in turn be blocked on this thread
+ // finishing. Instead, exit fast via halt().
+ Runtime.getRuntime().halt(numericExitCode);
+ }
}
}
} catch (Throwable t) {
@@ -239,38 +194,29 @@
+ " and include the information below.");
System.err.println("Original uncaught exception:");
- throwable.printStackTrace(System.err);
+ crash.getThrowable().printStackTrace(System.err);
System.err.println("Exception encountered during BugReport#handleCrash:");
t.printStackTrace(System.err);
} finally {
- logger.atInfo().log("Entered outer finally block");
- Runtime.getRuntime().halt(numericExitCode);
- logger.atSevere().log("Failed to halt (outer block)!");
+ if (ctx.shouldHaltJvm()) {
+ Runtime.getRuntime().halt(numericExitCode);
+ }
+ }
+ if (!ctx.shouldHaltJvm()) {
+ return;
}
logger.atSevere().log("Failed to crash in handleCrash");
- throw new IllegalStateException("never get here", throwable);
+ throw new IllegalStateException("Should have halted", crash.getThrowable());
}
- private static void printThrowableTo(OutErr outErr, Throwable e) {
- PrintStream err = new PrintStream(outErr.getErrorStream());
- e.printStackTrace(err);
- err.flush();
- logger.atSevere().withCause(e).log("%s crashed", getProductName());
- }
-
- /**
- * Print user-helpful information about the bug/crash to the output.
- *
- * @param outErr where to write the output
- * @param e the exception thrown
- */
- public static void printBug(OutErr outErr, Throwable e, @Nullable String oomMessage) {
- if (e instanceof OutOfMemoryError) {
- outErr.printErr("ERROR: " + constructOomExitMessage(oomMessage));
- } else {
- printThrowableTo(outErr, e);
- }
+ /** Constructs a user-helpful message for a crash bug. */
+ private static String constructCrashMessageWithStackTrace(Throwable throwable, CrashContext ctx) {
+ String msg =
+ throwable instanceof OutOfMemoryError
+ ? constructOomExitMessage(ctx.getExtraOomInfo())
+ : getProductName() + " crashed due to an internal error.";
+ return msg + " Printing stack trace:\n" + Throwables.getStackTraceAsString(throwable);
}
public static String constructOomExitMessage(@Nullable String extraInfo) {
@@ -286,7 +232,7 @@
* <li>{@code --default_override} is spammy.
* </ul>
*/
- private static List<String> filterArgs(Iterable<String> args) {
+ private static ImmutableList<String> filterArgs(Iterable<String> args) {
if (args == null) {
return null;
}
@@ -302,12 +248,12 @@
return filteredArgs.build();
}
- // Log the exception. Because this method is only called in a blaze release,
- // this will result in a report being sent to a remote logging service.
+ // Log the exception. Because this method is only called in a blaze release, this will result in a
+ // report being sent to a remote logging service.
private static void logException(Throwable exception, List<String> args, String... values) {
logger.atSevere().withCause(exception).log("Exception");
- // The preamble is used in the crash watcher, so don't change it
- // unless you know what you're doing.
+ // The preamble is used in the crash watcher, so don't change it unless you know what you're
+ // doing.
String preamble =
getProductName()
+ (exception instanceof OutOfMemoryError ? " OOMError: " : " crashed with args: ");
@@ -315,7 +261,8 @@
LoggingUtil.logToRemote(Level.SEVERE, preamble + Joiner.on(' ').join(args), exception, values);
}
- private static class DefaultBugReporter implements BugReporter {
+ private static final class DefaultBugReporter implements BugReporter {
+
@Override
public void sendBugReport(Throwable exception) {
BugReport.sendBugReport(exception);
@@ -327,8 +274,8 @@
}
@Override
- public RuntimeException handleCrash(Throwable throwable, String... args) {
- throw BugReport.handleCrash(throwable, args);
+ public void handleCrash(Crash crash, CrashContext ctx) {
+ BugReport.handleCrash(crash, ctx);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/bugreport/BugReporter.java b/src/main/java/com/google/devtools/build/lib/bugreport/BugReporter.java
index 3fc8ceac..03066a1 100644
--- a/src/main/java/com/google/devtools/build/lib/bugreport/BugReporter.java
+++ b/src/main/java/com/google/devtools/build/lib/bugreport/BugReporter.java
@@ -35,5 +35,5 @@
void sendBugReport(Throwable exception, List<String> args, String... values);
/** See {@link BugReport#handleCrash}. */
- RuntimeException handleCrash(Throwable throwable, String... args);
+ void handleCrash(Crash crash, CrashContext ctx);
}
diff --git a/src/main/java/com/google/devtools/build/lib/bugreport/Crash.java b/src/main/java/com/google/devtools/build/lib/bugreport/Crash.java
new file mode 100644
index 0000000..2205015
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bugreport/Crash.java
@@ -0,0 +1,64 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.bugreport;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import com.google.common.base.MoreObjects;
+import com.google.devtools.build.lib.util.CrashFailureDetails;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
+
+/** Encapsulates the {@link Throwable} and {@link DetailedExitCode} for a crash. */
+public final class Crash {
+
+ /**
+ * Creates a crash caused by the given {@link Throwable}.
+ *
+ * <p>The exit code is generated by {@link CrashFailureDetails#detailedExitCodeForThrowable}.
+ */
+ public static Crash from(Throwable throwable) {
+ return new Crash(throwable, CrashFailureDetails.detailedExitCodeForThrowable(throwable));
+ }
+
+ /** Creates a crash caused by the given {@link Throwable} with a specified {@link ExitCode}. */
+ public static Crash from(Throwable throwable, ExitCode exitCode) {
+ return new Crash(
+ throwable, DetailedExitCode.of(exitCode, CrashFailureDetails.forThrowable(throwable)));
+ }
+
+ private final Throwable throwable;
+ private final DetailedExitCode detailedExitCode;
+
+ private Crash(Throwable throwable, DetailedExitCode detailedExitCode) {
+ this.throwable = checkNotNull(throwable);
+ this.detailedExitCode = checkNotNull(detailedExitCode);
+ }
+
+ public Throwable getThrowable() {
+ return throwable;
+ }
+
+ public DetailedExitCode getDetailedExitCode() {
+ return detailedExitCode;
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("throwable", throwable)
+ .add("detailedExitCode", detailedExitCode)
+ .toString();
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/bugreport/CrashContext.java b/src/main/java/com/google/devtools/build/lib/bugreport/CrashContext.java
new file mode 100644
index 0000000..9b5a438
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/bugreport/CrashContext.java
@@ -0,0 +1,125 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.bugreport;
+
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.EventKind;
+import java.util.List;
+
+/** Context describing when a {@link Crash} occurred and how it should be handled. */
+public final class CrashContext {
+
+ /**
+ * Creates a {@link CrashContext} that instructs {@link BugReporter} to halt the JVM when handling
+ * a crash.
+ *
+ * <p>This should only be used when it is not feasible to conduct an orderly shutdown, for example
+ * a crash in an async thread.
+ */
+ public static CrashContext halt() {
+ return new CrashContext(/*haltJvm=*/ true);
+ }
+
+ /**
+ * Creates a {@link CrashContext} that instructs {@link BugReporter} <em>not</em> to halt the JVM
+ * when handling a crash.
+ *
+ * <p>The caller is responsible for terminating the server with an appropriate exit code.
+ */
+ public static CrashContext keepAlive() {
+ return new CrashContext(/*haltJvm=*/ false);
+ }
+
+ private final boolean haltJvm;
+ private ImmutableList<String> args = ImmutableList.of();
+ private boolean sendBugReport = true;
+ private String extraOomInfo = "";
+ private EventHandler eventHandler =
+ event -> System.err.println(event.getKind() + ": " + event.getMessage());
+
+ private CrashContext(boolean haltJvm) {
+ this.haltJvm = haltJvm;
+ }
+
+ /** Sets the arguments that {@link BugReporter} should include with the bug report. */
+ public CrashContext withArgs(String... args) {
+ this.args = ImmutableList.copyOf(args);
+ return this;
+ }
+
+ /** Sets the arguments that {@link BugReporter} should include with the bug report. */
+ public CrashContext withArgs(List<String> args) {
+ this.args = ImmutableList.copyOf(args);
+ return this;
+ }
+
+ /** Disables bug reporting. */
+ public CrashContext withoutBugReport() {
+ sendBugReport = false;
+ return this;
+ }
+
+ /**
+ * Sets a custom additional message that should be including when handling an {@link
+ * OutOfMemoryError}.
+ */
+ public CrashContext withExtraOomInfo(String extraOomInfo) {
+ this.extraOomInfo = extraOomInfo;
+ return this;
+ }
+
+ /**
+ * Sets the {@link EventHandler} that should be notified about the {@link EventKind#FATAL} crash
+ * event.
+ *
+ * <p>If this method is not called, the event is printed to {@link System#err}.
+ */
+ public CrashContext reportingTo(EventHandler eventHandler) {
+ this.eventHandler = eventHandler;
+ return this;
+ }
+
+ boolean shouldHaltJvm() {
+ return haltJvm;
+ }
+
+ ImmutableList<String> getArgs() {
+ return args;
+ }
+
+ boolean shouldSendBugReport() {
+ return sendBugReport;
+ }
+
+ String getExtraOomInfo() {
+ return extraOomInfo;
+ }
+
+ EventHandler getEventHandler() {
+ return eventHandler;
+ }
+
+ @Override
+ public String toString() {
+ return MoreObjects.toStringHelper(this)
+ .add("haltJvm", haltJvm)
+ .add("args", args)
+ .add("sendBugReport", sendBugReport)
+ .add("extraOomInfo", extraOomInfo)
+ .add("eventHandler", eventHandler)
+ .toString();
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
index b28b1ca..137bfa6 100644
--- a/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
+++ b/src/main/java/com/google/devtools/build/lib/buildeventservice/BuildEventServiceUploader.java
@@ -39,6 +39,7 @@
import com.google.devtools.build.lib.buildeventservice.BuildEventServiceUploaderCommands.StreamCompleteCommand;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient;
import com.google.devtools.build.lib.buildeventservice.client.BuildEventServiceClient.StreamContext;
+import com.google.devtools.build.lib.buildeventstream.AbortedEvent;
import com.google.devtools.build.lib.buildeventstream.ArtifactGroupNamer;
import com.google.devtools.build.lib.buildeventstream.BuildCompletingEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
@@ -193,7 +194,15 @@
}
// BuildCompletingEvent marks the end of the build in the BEP event stream.
if (event instanceof BuildCompletingEvent) {
- this.buildStatus = extractBuildStatus((BuildCompletingEvent) event);
+ ExitCode exitCode = ((BuildCompletingEvent) event).getExitCode();
+ if (exitCode != null && exitCode.getNumericExitCode() == 0) {
+ buildStatus = COMMAND_SUCCEEDED;
+ } else {
+ buildStatus = COMMAND_FAILED;
+ }
+ } else if (event instanceof AbortedEvent && event.getEventId().hasBuildFinished()) {
+ // An AbortedEvent with a build finished ID means we are crashing.
+ buildStatus = COMMAND_FAILED;
}
ensureUploadThreadStarted();
@@ -342,8 +351,7 @@
}
private BuildEventStreamProtos.BuildEvent createSerializedRegularBuildEvent(
- PathConverter pathConverter,
- SendRegularBuildEventCommand buildEvent) {
+ PathConverter pathConverter, SendRegularBuildEventCommand buildEvent) {
BuildEventContext ctx =
new BuildEventContext() {
@Override
@@ -361,8 +369,7 @@
return buildEventProtocolOptions;
}
};
- BuildEventStreamProtos.BuildEvent serializedBepEvent =
- buildEvent.getEvent().asStreamProto(ctx);
+ BuildEventStreamProtos.BuildEvent serializedBepEvent = buildEvent.getEvent().asStreamProto(ctx);
// TODO(lpino): Remove this logging once we can make every single event smaller than 1MB
// as protobuf recommends.
@@ -553,7 +560,7 @@
}
acksReceived = 0;
eventQueue.addFirst(new OpenStreamCommand());
- }
+ }
break;
}
}
@@ -663,14 +670,6 @@
return Timestamps.fromMillis(clock.currentTimeMillis());
}
- private static Result extractBuildStatus(BuildCompletingEvent event) {
- if (event.getExitCode() != null && event.getExitCode().getNumericExitCode() == 0) {
- return COMMAND_SUCCEEDED;
- } else {
- return COMMAND_FAILED;
- }
- }
-
private static Status lastEventNotSentStatus() {
return Status.FAILED_PRECONDITION.withDescription(
"Server closed stream with status OK but not all events have been sent");
@@ -809,4 +808,3 @@
}
}
}
-
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
index d418563..91d48f8 100644
--- a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
+++ b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -147,10 +147,12 @@
// Error out early if multi_cpus is set, but we're not in build or test command.
if (!request.getMultiCpus().isEmpty()) {
- getReporter().handle(Event.warn(
- "The --experimental_multi_cpu option is _very_ experimental and only intended for "
- + "internal testing at this time. If you do not work on the build tool, then you "
- + "should stop now!"));
+ getReporter()
+ .handle(
+ Event.warn(
+ "The --experimental_multi_cpu option is _very_ experimental and only intended"
+ + " for internal testing at this time. If you do not work on the build"
+ + " tool, then you should stop now!"));
if (!"build".equals(request.getCommandName()) && !"test".equals(request.getCommandName())) {
throw new InvalidConfigurationException(
"The experimental setting to select multiple CPUs is only supported for 'build' and "
@@ -235,11 +237,7 @@
}
Profiler.instance().markPhase(ProfilePhase.FINISH);
} catch (Error | RuntimeException e) {
- request
- .getOutErr()
- .printErrLn(
- "Internal error thrown during build. Printing stack trace: "
- + Throwables.getStackTraceAsString(e));
+ // Don't handle the error here. We will do so in stopRequest.
catastrophe = true;
throw e;
} finally {
@@ -385,21 +383,20 @@
*
* <p>The caller is responsible for setting up and syncing the package cache.
*
- * <p>During this function's execution, the actualTargets and successfulTargets
- * fields of the request object are set.
+ * <p>During this function's execution, the actualTargets and successfulTargets fields of the
+ * request object are set.
*
* @param request the build request that this build tool is servicing, which specifies various
- * options; during this method's execution, the actualTargets and successfulTargets fields
- * of the request object are populated
+ * options; during this method's execution, the actualTargets and successfulTargets fields of
+ * the request object are populated
* @param validator target validator
* @return the result as a {@link BuildResult} object
*/
- public BuildResult processRequest(
- BuildRequest request, TargetValidator validator) {
+ public BuildResult processRequest(BuildRequest request, TargetValidator validator) {
BuildResult result = new BuildResult(request.getStartTime());
maybeSetStopOnFirstFailure(request, result);
int startSuspendCount = suspendCount();
- Throwable catastrophe = null;
+ Throwable crash = null;
DetailedExitCode detailedExitCode = null;
try {
buildTargets(request, result, validator);
@@ -468,34 +465,33 @@
.build());
reportExceptionError(e);
} catch (Throwable throwable) {
- detailedExitCode = CrashFailureDetails.detailedExitCodeForThrowable(throwable);
- catastrophe = throwable;
- Throwables.propagate(throwable);
+ crash = throwable;
+ detailedExitCode = CrashFailureDetails.detailedExitCodeForThrowable(crash);
+ Throwables.throwIfUnchecked(throwable);
+ throw new IllegalStateException(throwable);
} finally {
if (detailedExitCode == null) {
detailedExitCode =
CrashFailureDetails.detailedExitCodeForThrowable(
new IllegalStateException("Unspecified DetailedExitCode"));
}
- stopRequest(result, catastrophe, detailedExitCode, startSuspendCount);
+ stopRequest(result, crash, detailedExitCode, startSuspendCount);
}
return result;
}
- private void maybeSetStopOnFirstFailure(BuildRequest request, BuildResult result) {
+ private static void maybeSetStopOnFirstFailure(BuildRequest request, BuildResult result) {
if (shouldStopOnFailure(request)) {
result.setStopOnFirstFailure(true);
}
}
- private boolean shouldStopOnFailure(BuildRequest request) {
+ private static boolean shouldStopOnFailure(BuildRequest request) {
return !(request.getKeepGoing() && request.getExecutionOptions().testKeepGoing);
}
- /**
- * Initializes the output filter to the value given with {@code --output_filter}.
- */
+ /** Initializes the output filter to the value given with {@code --output_filter}. */
private void initializeOutputFilter(BuildRequest request) {
RegexPatternOption outputFilterOption = request.getBuildOptions().outputFilter;
if (outputFilterOption != null) {
@@ -515,14 +511,14 @@
* <p>This logs the build result, cleans up and stops the clock.
*
* @param result result to update
- * @param crash any unexpected {@link RuntimeException} or {@link Error}. May be null
+ * @param crash any unexpected {@link RuntimeException} or {@link Error}, may be null
* @param detailedExitCode describes the exit code and an optional detailed failure value to add
* to {@code result}
* @param startSuspendCount number of suspensions before the build started
*/
public void stopRequest(
BuildResult result,
- Throwable crash,
+ @Nullable Throwable crash,
DetailedExitCode detailedExitCode,
int startSuspendCount) {
Preconditions.checkState((crash == null) || !detailedExitCode.isSuccess());
@@ -530,6 +526,7 @@
Preconditions.checkState(startSuspendCount <= stopSuspendCount);
result.setUnhandledThrowable(crash);
result.setDetailedExitCode(detailedExitCode);
+
InterruptedException ie = null;
try {
env.getSkyframeExecutor().notifyCommandComplete(env.getReporter());
@@ -541,13 +538,17 @@
result.setStopTime(runtime.getClock().currentTimeMillis());
result.setWasSuspended(stopSuspendCount > startSuspendCount);
- env.getEventBus().post(new BuildPrecompleteEvent());
- env.getEventBus()
- .post(
- new BuildCompleteEvent(
- result,
- ImmutableList.of(
- BuildEventIdUtil.buildToolLogs(), BuildEventIdUtil.buildMetrics())));
+ // Skip the build complete events so that modules can run blazeShutdownOnCrash without thinking
+ // that the build completed normally. BlazeCommandDispatcher will call handleCrash.
+ if (crash == null) {
+ env.getEventBus().post(new BuildPrecompleteEvent());
+ env.getEventBus()
+ .post(
+ new BuildCompleteEvent(
+ result,
+ ImmutableList.of(
+ BuildEventIdUtil.buildToolLogs(), BuildEventIdUtil.buildMetrics())));
+ }
// Post the build tool logs event; the corresponding local files may be contributed from
// modules, and this has to happen after posting the BuildCompleteEvent because that's when
// modules add their data to the collection.
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
index 5546df0..1a40e07 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/BUILD
@@ -35,7 +35,6 @@
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
- "//third_party:error_prone_annotations",
"//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
diff --git a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
index 196291d..8e725e5 100644
--- a/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
+++ b/src/main/java/com/google/devtools/build/lib/collect/nestedset/NestedSet.java
@@ -21,6 +21,8 @@
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.ListenableFuture;
import com.google.devtools.build.lib.bugreport.BugReport;
+import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.collect.compacthashset.CompactHashSet;
import com.google.devtools.build.lib.collect.nestedset.NestedSetStore.MissingNestedSetException;
import com.google.devtools.build.lib.concurrent.MoreFutures;
@@ -293,8 +295,8 @@
System.err.println(
"An interrupted exception occurred during nested set deserialization, "
+ "exiting abruptly.");
- BugReport.handleCrash(e, ExitCode.INTERRUPTED);
- throw new IllegalStateException("Server should have shut down.", e);
+ BugReport.handleCrash(Crash.from(e, ExitCode.INTERRUPTED), CrashContext.halt());
+ throw new IllegalStateException("Should have halted", e);
}
} else {
return children;
diff --git a/src/main/java/com/google/devtools/build/lib/events/Event.java b/src/main/java/com/google/devtools/build/lib/events/Event.java
index a7f4f94..d201009 100644
--- a/src/main/java/com/google/devtools/build/lib/events/Event.java
+++ b/src/main/java/com/google/devtools/build/lib/events/Event.java
@@ -46,7 +46,6 @@
*/
@Immutable
public final class Event implements Serializable {
- private int hashCode;
private final EventKind kind;
@@ -66,6 +65,8 @@
*/
private final ImmutableClassToInstanceMap<Object> properties;
+ private int hashCode;
+
private Event(EventKind kind, Object message, ImmutableClassToInstanceMap<Object> properties) {
this.kind = checkNotNull(kind);
this.message = checkNotNull(message);
@@ -356,6 +357,11 @@
: of(kind, messageBytes, Location.class, location);
}
+ /** Constructs an event with kind {@link EventKind#FATAL}. */
+ public static Event fatal(String message) {
+ return of(EventKind.FATAL, message);
+ }
+
/** Constructs an event with kind {@link EventKind#ERROR}, with an optional {@link Location}. */
public static Event error(@Nullable Location location, String message) {
return location == null
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index c8d3985..7e43f37 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -31,8 +31,9 @@
import com.google.common.io.Flushables;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.devtools.build.lib.analysis.NoBuildEvent;
-import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
+import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
import com.google.devtools.build.lib.buildtool.buildevent.ProfilerStartedEvent;
import com.google.devtools.build.lib.clock.BlazeClock;
@@ -202,8 +203,7 @@
currentClientDescription);
outErr.printErrLn(message);
return createDetailedCommandResult(
- message,
- FailureDetails.Command.Code.ANOTHER_COMMAND_RUNNING);
+ message, FailureDetails.Command.Code.ANOTHER_COMMAND_RUNNING);
default:
throw new IllegalStateException();
@@ -225,8 +225,7 @@
String message = "Server shut down " + shutdownReason;
outErr.printErrLn(message);
return createDetailedCommandResult(
- message,
- FailureDetails.Command.Code.PREVIOUSLY_SHUTDOWN);
+ message, FailureDetails.Command.Code.PREVIOUSLY_SHUTDOWN);
}
BlazeCommandResult result =
execExclusively(
@@ -352,8 +351,7 @@
String message = "Starlark CPU profiler: " + ex.getMessage();
outErr.printErrLn(message);
return createDetailedCommandResult(
- message,
- FailureDetails.Command.Code.STARLARK_CPU_PROFILE_FILE_INITIALIZATION_FAILURE);
+ message, FailureDetails.Command.Code.STARLARK_CPU_PROFILE_FILE_INITIALIZATION_FAILURE);
}
try {
Starlark.startCpuProfile(out, Duration.ofMillis(10));
@@ -361,18 +359,17 @@
String message = Strings.nullToEmpty(ex.getMessage());
outErr.printErrLn(message);
return createDetailedCommandResult(
- message,
- FailureDetails.Command.Code.STARLARK_CPU_PROFILING_INITIALIZATION_FAILURE);
+ message, FailureDetails.Command.Code.STARLARK_CPU_PROFILING_INITIALIZATION_FAILURE);
}
}
BlazeCommandResult result =
createDetailedCommandResult(
- "Unknown command failure",
- FailureDetails.Command.Code.COMMAND_FAILURE_UNKNOWN);
- boolean afterCommandCalled = false;
+ "Unknown command failure", FailureDetails.Command.Code.COMMAND_FAILURE_UNKNOWN);
+ boolean needToCallAfterCommand = true;
Reporter reporter = env.getReporter();
- try (OutErr.SystemPatcher systemOutErrPatcher = reporter.getOutErr().getSystemPatcher()) {
+ OutErr.SystemPatcher systemOutErrPatcher = reporter.getOutErr().getSystemPatcher();
+ try {
// Temporary: there are modules that output events during beforeCommand, but the reporter
// isn't setup yet. Add the stored event handler to catch those events.
reporter.addHandler(storedEventHandler);
@@ -598,26 +595,27 @@
if (result.getDetailedExitCode().isSuccess()) { // don't clobber existing error
result =
createDetailedCommandResult(
- message,
- FailureDetails.Command.Code.STARLARK_CPU_PROFILE_FILE_WRITE_FAILURE);
+ message, FailureDetails.Command.Code.STARLARK_CPU_PROFILE_FILE_WRITE_FAILURE);
}
}
}
- afterCommandCalled = true;
+ needToCallAfterCommand = false;
return runtime.afterCommand(env, result);
} catch (Throwable e) {
- outErr.printErr(
- "Internal error thrown during build. Printing stack trace: "
- + Throwables.getStackTraceAsString(e));
- e.printStackTrace();
- BugReport.printBug(outErr, e, commonOptions.oomMessage);
- bugReporter.sendBugReport(e, args);
logger.atSevere().withCause(e).log("Shutting down due to exception");
- result = BlazeCommandResult.createShutdown(e);
+ Crash crash = Crash.from(e);
+ bugReporter.handleCrash(
+ crash,
+ CrashContext.keepAlive()
+ .withArgs(args)
+ .withExtraOomInfo(commonOptions.oomMessage)
+ .reportingTo(reporter));
+ needToCallAfterCommand = false; // We are crashing.
+ result = BlazeCommandResult.createShutdown(crash);
return result;
} finally {
- if (!afterCommandCalled) {
+ if (needToCallAfterCommand) {
BlazeCommandResult newResult = runtime.afterCommand(env, result);
if (!newResult.equals(result)) {
logger.atWarning().log("afterCommand yielded different result: %s %s", result, newResult);
@@ -627,6 +625,8 @@
Flushables.flushQuietly(outErr.getOutputStream());
Flushables.flushQuietly(outErr.getErrorStream());
+ systemOutErrPatcher.close();
+
env.getTimestampGranularityMonitor().waitForTimestampGranularity(outErr);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
index 293a76a..f4b7003 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandResult.java
@@ -16,10 +16,10 @@
import com.google.common.base.MoreObjects;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.bugreport.Crash;
import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
import com.google.devtools.build.lib.server.CommandProtos.ExecRequest;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
-import com.google.devtools.build.lib.util.CrashFailureDetails;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
import javax.annotation.Nullable;
@@ -83,8 +83,8 @@
return new BlazeCommandResult(DetailedExitCode.success(), null, true);
}
- static BlazeCommandResult createShutdown(Throwable e) {
- return new BlazeCommandResult(CrashFailureDetails.detailedExitCodeForThrowable(e), null, true);
+ static BlazeCommandResult createShutdown(Crash crash) {
+ return new BlazeCommandResult(crash.getDetailedExitCode(), null, true);
}
public static BlazeCommandResult success() {
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index 1e2503f..3ad29cb 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -39,6 +39,8 @@
import com.google.devtools.build.lib.bazel.repository.downloader.Downloader;
import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
+import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.buildeventstream.BuildEvent.LocalFile.LocalFileType;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader;
import com.google.devtools.build.lib.buildeventstream.BuildEventArtifactUploader.UploadContext;
@@ -83,7 +85,6 @@
import com.google.devtools.build.lib.shell.SubprocessBuilder;
import com.google.devtools.build.lib.shell.SubprocessFactory;
import com.google.devtools.build.lib.util.AbruptExitException;
-import com.google.devtools.build.lib.util.CrashFailureDetails;
import com.google.devtools.build.lib.util.CustomExitCodePublisher;
import com.google.devtools.build.lib.util.CustomFailureDetailPublisher;
import com.google.devtools.build.lib.util.DebugLoggerConfigurator;
@@ -738,7 +739,7 @@
}
}
- /** Invokes {@link BlazeModule#blazeShutdownOnCrash()} on all registered modules. */
+ /** Invokes {@link BlazeModule#blazeShutdownOnCrash} on all registered modules. */
private void shutDownModulesOnCrash(DetailedExitCode exitCode) {
// TODO(b/167592709): remove verbose logging when bug resolved.
logger.atInfo().log("Shutting down modules on crash: %s", blazeModules);
@@ -775,11 +776,9 @@
// Run Blaze in server mode.
System.exit(serverMain(modules, OutErr.SYSTEM_OUT_ERR, args));
} catch (RuntimeException | Error e) { // A definite bug...
- BugReport.printBug(OutErr.SYSTEM_OUT_ERR, e, /* oomMessage = */ null);
- BugReport.sendBugReport(e, Arrays.asList(args));
- CustomFailureDetailPublisher.maybeWriteFailureDetailFile(CrashFailureDetails.forThrowable(e));
- System.exit(ExitCode.BLAZE_INTERNAL_ERROR.getNumericExitCode());
- throw e; // Shouldn't get here.
+ Crash crash = Crash.from(e);
+ BugReport.handleCrash(crash, CrashContext.keepAlive().withArgs(args));
+ System.exit(crash.getDetailedExitCode().getExitCode().getNumericExitCode());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
index 31fc9c3..d38bd56 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BuildEventStreamer.java
@@ -42,6 +42,7 @@
import com.google.devtools.build.lib.buildeventstream.BuildCompletingEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEvent;
import com.google.devtools.build.lib.buildeventstream.BuildEventIdUtil;
+import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Aborted;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.Aborted.AbortReason;
import com.google.devtools.build.lib.buildeventstream.BuildEventStreamProtos.BuildEventId;
import com.google.devtools.build.lib.buildeventstream.BuildEventTransport;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java b/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java
index 8ba6475..b5997b3 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiter.java
@@ -21,10 +21,10 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.GoogleLogger;
-import com.google.devtools.build.lib.bugreport.BugReport;
import com.google.devtools.build.lib.bugreport.BugReporter;
+import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.concurrent.ThreadSafety;
-import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.server.FailureDetails;
@@ -208,20 +208,19 @@
if (percentUsed > occupiedHeapPercentageThreshold.getAsInt()) {
if (info.getGcCause().equals("System.gc()") && !throwingOom.getAndSet(true)) {
// Assume we got here from a GC initiated by the other branch.
- String exitMsg = BugReport.constructOomExitMessage(oomMessage);
- eventHandler.handle(Event.error(exitMsg));
- String detailedExitMsg =
- String.format(
- "%s RetainedHeapLimiter forcing exit due to GC thrashing: After back-to-back"
- + " full GCs, the tenured space is more than %s%% occupied (%s out of"
- + " a tenured space size of %s).",
- exitMsg,
- occupiedHeapPercentageThreshold.getAsInt(),
- space.getUsed(),
- space.getMax());
- logger.atSevere().log(detailedExitMsg);
+ OutOfMemoryError oom =
+ new OutOfMemoryError(
+ String.format(
+ "RetainedHeapLimiter forcing exit due to GC thrashing: After back-to-back"
+ + " full GCs, the tenured space is more than %s%% occupied (%s out of"
+ + " a tenured space size of %s).",
+ occupiedHeapPercentageThreshold.getAsInt(),
+ space.getUsed(),
+ space.getMax()));
// Exits the runtime.
- throw bugReporter.handleCrash(new OutOfMemoryError(detailedExitMsg));
+ bugReporter.handleCrash(
+ Crash.from(oom),
+ CrashContext.halt().withExtraOomInfo(oomMessage).reportingTo(eventHandler));
}
if (System.currentTimeMillis() - lastTriggeredGcInMilliseconds.get()
diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD
index b7a8dc7..c5fa44a 100644
--- a/src/main/java/com/google/devtools/build/lib/util/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/util/BUILD
@@ -232,6 +232,7 @@
"ExitCode.java",
],
deps = [
+ "//third_party:error_prone_annotations",
"//third_party:guava",
"//third_party:jsr305",
],
diff --git a/src/main/java/com/google/devtools/build/lib/util/ExitCode.java b/src/main/java/com/google/devtools/build/lib/util/ExitCode.java
index 3e69108..c821a24 100644
--- a/src/main/java/com/google/devtools/build/lib/util/ExitCode.java
+++ b/src/main/java/com/google/devtools/build/lib/util/ExitCode.java
@@ -15,27 +15,27 @@
package com.google.devtools.build.lib.util;
import com.google.common.base.Objects;
+import com.google.errorprone.annotations.Immutable;
import java.util.Collection;
import java.util.HashMap;
import javax.annotation.Nullable;
/**
- * <p>Anything marked FAILURE is generally from a problem with the source code
- * under consideration. In these cases, a re-run in an identical client should
- * produce an identical return code all things being constant.
+ * Anything marked FAILURE is generally from a problem with the source code under consideration. In
+ * these cases, a re-run in an identical client should produce an identical return code all things
+ * being constant.
*
- * <p>Anything marked as an ERROR is generally a problem unrelated to the
- * source code itself. It is either something wrong with the user's command
- * line or the user's machine or environment.
+ * <p>Anything marked as an ERROR is generally a problem unrelated to the source code itself. It is
+ * either something wrong with the user's command line or the user's machine or environment.
*
- * <p>Note that these exit codes should be kept consistent with the codes
- * returned by Blaze's launcher in //devtools/blaze/main:blaze.cc
- * Blaze exit codes should be consistently classified as permanent vs.
- * transient (i.e. retriable) vs. unknown transient/permanent because users,
- * in particular infrastructure users, will use the exit code to decide whether
- * the request should be retried or not.
+ * <p>Note that these exit codes should be kept consistent with the codes returned by Blaze's
+ * launcher in //devtools/blaze/main:blaze.cc Blaze exit codes should be consistently classified as
+ * permanent vs. transient (i.e. retriable) vs. unknown transient/permanent because users, in
+ * particular infrastructure users, will use the exit code to decide whether the request should be
+ * retried or not.
*/
-public class ExitCode {
+@Immutable
+public final class ExitCode {
// Tracks all exit codes defined here and elsewhere in Bazel.
private static final HashMap<Integer, ExitCode> exitCodeRegistry = new HashMap<>();
diff --git a/src/main/java/com/google/devtools/build/lib/util/io/BUILD b/src/main/java/com/google/devtools/build/lib/util/io/BUILD
index 804b3c4..60b3782 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/util/io/BUILD
@@ -41,5 +41,8 @@
java_library(
name = "out-err",
srcs = OUT_ERR_SRCS,
- deps = ["//third_party:guava"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib/profiler",
+ "//third_party:guava",
+ ],
)
diff --git a/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java b/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
index 08942f4..4e8de9f 100644
--- a/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
+++ b/src/main/java/com/google/devtools/build/lib/util/io/OutErr.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.util.io;
import com.google.common.base.Preconditions;
+import com.google.devtools.build.lib.profiler.SilentCloseable;
import java.io.Closeable;
import java.io.IOException;
import java.io.OutputStream;
@@ -31,9 +32,7 @@
public static final OutErr SYSTEM_OUT_ERR = create(System.out, System.err);
- /**
- * Creates a new OutErr instance from the specified output and error streams.
- */
+ /** Creates a new OutErr instance from the specified output and error streams. */
public static OutErr create(OutputStream out, OutputStream err) {
return new OutErr(out, err);
}
@@ -62,7 +61,7 @@
* ends the scope of the patch, returning {@link System#out} and {@link System#err} to what they
* were before.
*/
- public interface SystemPatcher extends AutoCloseable {
+ public interface SystemPatcher extends SilentCloseable {
void start();
}
@@ -109,38 +108,38 @@
}
/**
- * Creates a new OutErr instance from the specified stream.
- * Writes to either the output or err of the new OutErr are written
- * to outputStream, synchronized.
+ * Creates a new OutErr instance from the specified stream. Writes to either the output or err of
+ * the new OutErr are written to outputStream, synchronized.
*/
public static OutErr createSynchronizedFunnel(final OutputStream outputStream) {
- OutputStream syncOut = new OutputStream() {
+ OutputStream syncOut =
+ new OutputStream() {
- @Override
- public synchronized void write(int b) throws IOException {
- outputStream.write(b);
- }
+ @Override
+ public synchronized void write(int b) throws IOException {
+ outputStream.write(b);
+ }
- @Override
- public synchronized void write(byte b[]) throws IOException {
- outputStream.write(b);
- }
+ @Override
+ public synchronized void write(byte[] b) throws IOException {
+ outputStream.write(b);
+ }
- @Override
- public synchronized void write(byte b[], int off, int len) throws IOException {
- outputStream.write(b, off, len);
- }
+ @Override
+ public synchronized void write(byte[] b, int off, int len) throws IOException {
+ outputStream.write(b, off, len);
+ }
- @Override
- public synchronized void flush() throws IOException {
- outputStream.flush();
- }
+ @Override
+ public synchronized void flush() throws IOException {
+ outputStream.flush();
+ }
- @Override
- public synchronized void close() throws IOException {
- outputStream.close();
- }
- };
+ @Override
+ public synchronized void close() throws IOException {
+ outputStream.close();
+ }
+ };
return create(syncOut, syncOut);
}
@@ -153,9 +152,7 @@
return err;
}
- /**
- * Writes the specified string to the output stream, and flushes.
- */
+ /** Writes the specified string to the output stream, and flushes. */
public void printOut(String s) {
PrintWriter writer = new PrintWriter(out, true);
writer.print(s);
@@ -166,9 +163,7 @@
printOut(s + "\n");
}
- /**
- * Writes the specified string to the error stream, and flushes.
- */
+ /** Writes the specified string to the error stream, and flushes. */
public void printErr(String s) {
PrintWriter writer = new PrintWriter(err, true);
writer.print(s);
@@ -178,5 +173,4 @@
public void printErrLn(String s) {
printErr(s + "\n");
}
-
}
diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD
index e8986c1..efc7807 100644
--- a/src/test/java/com/google/devtools/build/lib/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/BUILD
@@ -406,6 +406,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:execution_options",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
+ "//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/query2",
"//src/main/java/com/google/devtools/build/lib/query2/common:abstract-blaze-query-env",
"//src/main/java/com/google/devtools/build/lib/query2/engine",
diff --git a/src/test/java/com/google/devtools/build/lib/bugreport/BugReportTest.java b/src/test/java/com/google/devtools/build/lib/bugreport/BugReportTest.java
index 4199c3e..41d62e4 100644
--- a/src/test/java/com/google/devtools/build/lib/bugreport/BugReportTest.java
+++ b/src/test/java/com/google/devtools/build/lib/bugreport/BugReportTest.java
@@ -20,36 +20,90 @@
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import com.google.common.base.Throwables;
+import com.google.common.collect.Lists;
import com.google.devtools.build.lib.bugreport.BugReport.BlazeRuntimeInterface;
-import com.google.devtools.build.lib.server.FailureDetails.Crash;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.EventKind;
+import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.Crash.Code;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
-import com.google.devtools.build.lib.server.FailureDetails.Throwable;
import com.google.devtools.build.lib.util.CustomExitCodePublisher;
import com.google.devtools.build.lib.util.CustomFailureDetailPublisher;
import com.google.devtools.build.lib.util.DetailedExitCode;
import com.google.devtools.build.lib.util.ExitCode;
import com.google.protobuf.ExtensionRegistry;
-import java.io.File;
-import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.util.List;
+import java.util.Arrays;
import org.junit.After;
+import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import org.junit.runner.RunWith;
-import org.junit.runners.JUnit4;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
import org.mockito.ArgumentCaptor;
/** Tests for {@link BugReport}. */
-@RunWith(JUnit4.class)
-public class BugReportTest {
+@RunWith(Parameterized.class)
+public final class BugReportTest {
- private static final String TEST_EXCEPTION_NAME =
- "com.google.devtools.build.lib.bugreport.BugReportTest$TestException";
- @Rule public final TemporaryFolder temporaryFolder = new TemporaryFolder();
+ private enum CrashType {
+ CRASH(ExitCode.BLAZE_INTERNAL_ERROR, Code.CRASH_UNKNOWN) {
+ @Override
+ Throwable createThrowable() {
+ return new IllegalStateException("Crashed");
+ }
+ },
+ OOM(ExitCode.OOM_ERROR, Code.CRASH_OOM) {
+ @Override
+ Throwable createThrowable() {
+ return new OutOfMemoryError("Java heap space");
+ }
+ };
+
+ private final ExitCode expectedExitCode;
+ private final Code expectedFailureDetailCode;
+
+ CrashType(ExitCode expectedExitCode, Code expectedFailureDetailCode) {
+ this.expectedExitCode = expectedExitCode;
+ this.expectedFailureDetailCode = expectedFailureDetailCode;
+ }
+
+ abstract Throwable createThrowable();
+ }
+
+ @Parameters
+ public static CrashType[] params() {
+ return CrashType.values();
+ }
+
+ @Rule public final TemporaryFolder tmp = new TemporaryFolder();
+
+ private final CrashType crashType;
+ private final BlazeRuntimeInterface mockRuntime = mock(BlazeRuntimeInterface.class);
+
+ private Path exitCodeFile;
+ private Path failureDetailFile;
+
+ public BugReportTest(CrashType crashType) {
+ this.crashType = crashType;
+ }
+
+ @Before
+ public void setup() throws Exception {
+ when(mockRuntime.getProductName()).thenReturn("myProductName");
+ BugReport.setRuntime(mockRuntime);
+
+ exitCodeFile = tmp.newFolder().toPath().resolve("exit_code_to_use_on_abrupt_exit");
+ failureDetailFile = tmp.newFolder().toPath().resolve("failure_detail");
+
+ CustomExitCodePublisher.setAbruptExitStatusFileDir(exitCodeFile.getParent().toString());
+ CustomFailureDetailPublisher.setFailureDetailFilePath(failureDetailFile.toString());
+ }
@After
public void resetPublishers() {
@@ -58,60 +112,94 @@
}
@Test
- public void handleCrash() throws Exception {
- BlazeRuntimeInterface mockRuntime = mock(BlazeRuntimeInterface.class);
- when(mockRuntime.getProductName()).thenReturn("myProductName");
- BugReport.setRuntime(mockRuntime);
+ public void convenienceMethod() throws Exception {
+ Throwable t = crashType.createThrowable();
+ FailureDetail expectedFailureDetail =
+ createExpectedFailureDetail(t, crashType.expectedFailureDetailCode);
- File folderForExitStatusFile = temporaryFolder.newFolder();
- CustomExitCodePublisher.setAbruptExitStatusFileDir(folderForExitStatusFile.getPath());
+ assertThrows(SecurityException.class, () -> BugReport.handleCrash(t));
+ assertThrows(t.getClass(), BugReport::maybePropagateUnprocessedThrowableIfInTest);
- Path failureDetailFilePath =
- folderForExitStatusFile.toPath().resolve("failure_detail.rawproto");
- CustomFailureDetailPublisher.setFailureDetailFilePath(failureDetailFilePath.toString());
+ verify(mockRuntime)
+ .cleanUpForCrash(DetailedExitCode.of(crashType.expectedExitCode, expectedFailureDetail));
+ verifyExitCodeWritten(crashType.expectedExitCode.getNumericExitCode());
+ verifyFailureDetailWritten(expectedFailureDetail);
+ }
+
+ @Test
+ public void halt() throws Exception {
+ Throwable t = crashType.createThrowable();
+ FailureDetail expectedFailureDetail =
+ createExpectedFailureDetail(t, crashType.expectedFailureDetailCode);
assertThrows(
- SecurityException.class,
- () ->
- BugReport.handleCrashWithoutSendingBugReport(
- functionForStackFrameTest(), ExitCode.BLAZE_INTERNAL_ERROR));
+ SecurityException.class, () -> BugReport.handleCrash(Crash.from(t), CrashContext.halt()));
+ assertThrows(t.getClass(), BugReport::maybePropagateUnprocessedThrowableIfInTest);
- Path exitStatusFile =
- folderForExitStatusFile.toPath().resolve("exit_code_to_use_on_abrupt_exit");
- List<String> strings = Files.readAllLines(exitStatusFile, StandardCharsets.UTF_8);
- assertThat(strings).containsExactly("37");
-
- FailureDetail failureDetail =
- FailureDetail.parseFrom(
- Files.readAllBytes(failureDetailFilePath), ExtensionRegistry.getEmptyRegistry());
- assertThat(failureDetail.getMessage())
- .isEqualTo(String.format("Crashed: (%s) myMessage", TEST_EXCEPTION_NAME));
- assertThat(failureDetail.hasCrash()).isTrue();
- Crash crash = failureDetail.getCrash();
- assertThat(crash.getCode()).isEqualTo(Code.CRASH_UNKNOWN);
- assertThat(crash.getCausesList()).hasSize(1);
- Throwable cause = crash.getCauses(0);
- assertThat(cause.getMessage()).isEqualTo("myMessage");
- assertThat(cause.getThrowableClass()).isEqualTo(TEST_EXCEPTION_NAME);
- assertThat(cause.getStackTraceCount()).isAtLeast(1);
- assertThat(cause.getStackTrace(0))
- .contains(
- "com.google.devtools.build.lib.bugreport.BugReportTest.functionForStackFrameTest");
- ArgumentCaptor<DetailedExitCode> exitCodeCaptor =
- ArgumentCaptor.forClass(DetailedExitCode.class);
- verify(mockRuntime).cleanUpForCrash(exitCodeCaptor.capture());
- DetailedExitCode exitCode = exitCodeCaptor.getValue();
- assertThat(exitCode.getExitCode()).isEqualTo(ExitCode.BLAZE_INTERNAL_ERROR);
- assertThat(exitCode.getFailureDetail()).isEqualTo(failureDetail);
+ verify(mockRuntime)
+ .cleanUpForCrash(DetailedExitCode.of(crashType.expectedExitCode, expectedFailureDetail));
+ verifyExitCodeWritten(crashType.expectedExitCode.getNumericExitCode());
+ verifyFailureDetailWritten(expectedFailureDetail);
}
- private TestException functionForStackFrameTest() {
- return new TestException("myMessage");
+ @Test
+ public void keepAlive() throws Exception {
+ Throwable t = crashType.createThrowable();
+ FailureDetail expectedFailureDetail =
+ createExpectedFailureDetail(t, crashType.expectedFailureDetailCode);
+
+ BugReport.handleCrash(Crash.from(t), CrashContext.keepAlive());
+ assertThrows(t.getClass(), BugReport::maybePropagateUnprocessedThrowableIfInTest);
+
+ verify(mockRuntime)
+ .cleanUpForCrash(DetailedExitCode.of(crashType.expectedExitCode, expectedFailureDetail));
+ verifyNoExitCodeWritten();
+ verifyFailureDetailWritten(expectedFailureDetail);
}
- private static class TestException extends Exception {
- private TestException(String message) {
- super(message);
- }
+ @Test
+ public void customEventHandler() {
+ Throwable t = crashType.createThrowable();
+ EventHandler handler = mock(EventHandler.class);
+ ArgumentCaptor<Event> event = ArgumentCaptor.forClass(Event.class);
+
+ BugReport.handleCrash(Crash.from(t), CrashContext.keepAlive().reportingTo(handler));
+ assertThrows(t.getClass(), BugReport::maybePropagateUnprocessedThrowableIfInTest);
+
+ verify(handler).handle(event.capture());
+ assertThat(event.getValue().getKind()).isEqualTo(EventKind.FATAL);
+ assertThat(event.getValue().getMessage()).contains(Throwables.getStackTraceAsString(t));
+ }
+
+ private void verifyExitCodeWritten(int exitCode) throws Exception {
+ assertThat(Files.readAllLines(exitCodeFile)).containsExactly(String.valueOf(exitCode));
+ }
+
+ private void verifyNoExitCodeWritten() {
+ assertThat(exitCodeFile.toFile().exists()).isFalse();
+ }
+
+ private void verifyFailureDetailWritten(FailureDetail expected) throws Exception {
+ assertThat(
+ FailureDetail.parseFrom(
+ Files.readAllBytes(failureDetailFile), ExtensionRegistry.getEmptyRegistry()))
+ .isEqualTo(expected);
+ }
+
+ private static FailureDetail createExpectedFailureDetail(
+ Throwable t, Code expectedFailureDetailCode) {
+ return FailureDetail.newBuilder()
+ .setMessage(String.format("Crashed: (%s) %s", t.getClass().getName(), t.getMessage()))
+ .setCrash(
+ FailureDetails.Crash.newBuilder()
+ .setCode(expectedFailureDetailCode)
+ .addCauses(
+ FailureDetails.Throwable.newBuilder()
+ .setThrowableClass(t.getClass().getName())
+ .setMessage(t.getMessage())
+ .addAllStackTrace(
+ Lists.transform(
+ Arrays.asList(t.getStackTrace()), StackTraceElement::toString))))
+ .build();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD b/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD
index daa4253..162e4b1 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BUILD
@@ -50,6 +50,7 @@
"//src/main/java/com/google/devtools/build/lib/analysis:workspace_status_action",
"//src/main/java/com/google/devtools/build/lib/bazel:main",
"//src/main/java/com/google/devtools/build/lib/bazel:repository_module",
+ "//src/main/java/com/google/devtools/build/lib/bugreport",
"//src/main/java/com/google/devtools/build/lib/buildeventstream",
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/cmdline",
diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
index 7fc4dc8..1eee17c 100644
--- a/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
+++ b/src/test/java/com/google/devtools/build/lib/buildtool/util/BlazeRuntimeWrapper.java
@@ -14,6 +14,8 @@
package com.google.devtools.build.lib.buildtool.util;
+import static com.google.common.base.Preconditions.checkNotNull;
+
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -25,6 +27,8 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.ServerDirectories;
import com.google.devtools.build.lib.analysis.config.BuildConfigurationCollection;
+import com.google.devtools.build.lib.bugreport.Crash;
+import com.google.devtools.build.lib.bugreport.CrashContext;
import com.google.devtools.build.lib.buildeventstream.BuildEventProtocolOptions;
import com.google.devtools.build.lib.buildtool.BuildRequest;
import com.google.devtools.build.lib.buildtool.BuildRequestOptions;
@@ -65,18 +69,18 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParser;
import com.google.devtools.common.options.OptionsParsingException;
-import java.io.PrintStream;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
/**
- * A wrapper for {@link BlazeRuntime} for testing purposes that makes it possible to exercise
- * (most) of the build machinery in integration tests. Note that {@code BlazeCommandDispatcher}
- * is not exercised here.
+ * A wrapper for {@link BlazeRuntime} for testing purposes that makes it possible to exercise (most)
+ * of the build machinery in integration tests. Note that {@code BlazeCommandDispatcher} is not
+ * exercised here.
*/
public class BlazeRuntimeWrapper {
+
private final BlazeRuntime runtime;
private CommandEnvironment env;
private final EventCollectionApparatus events;
@@ -93,9 +97,12 @@
private final List<Object> eventBusSubscribers = new ArrayList<>();
public BlazeRuntimeWrapper(
- EventCollectionApparatus events, ServerDirectories serverDirectories,
- BlazeDirectories directories, BinTools binTools, BlazeRuntime.Builder builder)
- throws Exception {
+ EventCollectionApparatus events,
+ ServerDirectories serverDirectories,
+ BlazeDirectories directories,
+ BinTools binTools,
+ BlazeRuntime.Builder builder)
+ throws Exception {
this.events = events;
runtime =
builder
@@ -174,7 +181,7 @@
}
}
- public BlazeRuntime getRuntime() {
+ public final BlazeRuntime getRuntime() {
return runtime;
}
@@ -196,10 +203,9 @@
runtime.afterCommand(env, BlazeCommandResult.success());
}
- if (optionsParser == null) {
- throw new IllegalArgumentException("The options parser must be initialized before creating "
- + "a new command environment");
- }
+ checkNotNull(
+ optionsParser,
+ "The options parser must be initialized before creating a new command environment");
env =
runtime
@@ -245,7 +251,7 @@
return optionsParser.getOptions(optionsClass);
}
- protected void finalizeBuildResult(@SuppressWarnings("unused") BuildResult request) {}
+ void finalizeBuildResult(@SuppressWarnings("unused") BuildResult request) {}
/**
* Initializes a new options parser, parsing all the options set by {@link
@@ -266,31 +272,27 @@
}
commandCreated = false;
BuildTool buildTool = new BuildTool(env);
- PrintStream origSystemOut = System.out;
- PrintStream origSystemErr = System.err;
- try {
+ try (OutErr.SystemPatcher systemOutErrPatcher =
+ env.getReporter().getOutErr().getSystemPatcher()) {
Profiler.instance()
.start(
- ImmutableSet.of(),
- /* stream= */ null,
- /* format= */ null,
- /* outputBase= */ null,
- /* buildID= */ null,
- /* recordAllDurations= */ false,
+ /*profiledTasks=*/ ImmutableSet.of(),
+ /*stream=*/ null,
+ /*format=*/ null,
+ /*outputBase=*/ null,
+ /*buildID=*/ null,
+ /*recordAllDurations=*/ false,
new JavaClock(),
- /* execStartTimeNanos= */ 42,
- /* enabledCpuUsageProfiling= */ false,
- /* slimProfile= */ false,
- /* includePrimaryOutput= */ false,
- /* includeTargetLabel= */ false);
- OutErr outErr = env.getReporter().getOutErr();
- System.setOut(new PrintStream(outErr.getOutputStream(), /*autoflush=*/ true));
- System.setErr(new PrintStream(outErr.getErrorStream(), /*autoflush=*/ true));
+ /*execStartTimeNanos=*/ 42,
+ /*enabledCpuUsageProfiling=*/ false,
+ /*slimProfile=*/ false,
+ /*includePrimaryOutput=*/ false,
+ /*includeTargetLabel=*/ false);
// This cannot go into newCommand, because we hook up the EventCollectionApparatus as a
// module, and after that ran, further changes to the apparatus aren't reflected on the
// reporter.
- for (BlazeModule module : getRuntime().getBlazeModules()) {
+ for (BlazeModule module : runtime.getBlazeModules()) {
module.beforeCommand(env);
}
EventBus eventBus = env.getEventBus();
@@ -302,36 +304,40 @@
lastRequest = createRequest(env.getCommandName(), targets);
lastResult = new BuildResult(lastRequest.getStartTime());
- boolean success = false;
- for (BlazeModule module : getRuntime().getBlazeModules()) {
+ for (BlazeModule module : runtime.getBlazeModules()) {
env.getSkyframeExecutor().injectExtraPrecomputedValues(module.getPrecomputedValues());
}
+ Crash crash = null;
+ DetailedExitCode detailedExitCode = DetailedExitCode.of(createGenericDetailedFailure());
try {
try (SilentCloseable c = Profiler.instance().profile("syncPackageLoading")) {
env.syncPackageLoading(lastRequest);
}
buildTool.buildTargets(lastRequest, lastResult, null);
- success = true;
+ detailedExitCode = DetailedExitCode.success();
+ } catch (RuntimeException | Error e) {
+ crash = Crash.from(e);
+ detailedExitCode = crash.getDetailedExitCode();
+ throw e;
} finally {
- env
- .getTimestampGranularityMonitor()
- .waitForTimestampGranularity(lastRequest.getOutErr());
+ env.getTimestampGranularityMonitor().waitForTimestampGranularity(lastRequest.getOutErr());
this.configurations = lastResult.getBuildConfigurationCollection();
finalizeBuildResult(lastResult);
buildTool.stopRequest(
lastResult,
- null,
- success
- ? DetailedExitCode.success()
- : DetailedExitCode.of(createGenericDetailedFailure()),
+ crash != null ? crash.getThrowable() : null,
+ detailedExitCode,
/*startSuspendCount=*/ 0);
getSkyframeExecutor().notifyCommandComplete(env.getReporter());
+ if (crash != null) {
+ runtime
+ .getBugReporter()
+ .handleCrash(crash, CrashContext.keepAlive().reportingTo(env.getReporter()));
+ }
}
} finally {
- System.setOut(origSystemOut);
- System.setErr(origSystemErr);
Profiler.instance().stop();
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherTest.java b/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherTest.java
index 18fca18..68bc52c 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcherTest.java
@@ -16,7 +16,6 @@
import static com.google.common.truth.Truth.assertThat;
import static java.nio.charset.StandardCharsets.US_ASCII;
import static java.util.Arrays.asList;
-import static org.mockito.Mockito.mock;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
@@ -31,6 +30,8 @@
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventKind;
import com.google.devtools.build.lib.events.Reporter;
+import com.google.devtools.build.lib.profiler.MemoryProfiler;
+import com.google.devtools.build.lib.profiler.Profiler;
import com.google.devtools.build.lib.runtime.CommandDispatcher.LockingMode;
import com.google.devtools.build.lib.runtime.proto.InvocationPolicyOuterClass.InvocationPolicy;
import com.google.devtools.build.lib.server.FailureDetails;
@@ -66,6 +67,7 @@
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.function.Supplier;
+import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -130,6 +132,13 @@
errorOnAfterCommand = null;
}
+ @After
+ public void stopProfilers() throws Exception {
+ // Needs to be done because we are simulating crashes but keeping the jvm alive.
+ Profiler.instance().stop();
+ MemoryProfiler.instance().stop();
+ }
+
/** Options for {@link FooCommand}. */
public static class FooOptions extends OptionsBase {
@@ -248,8 +257,8 @@
throw new OutOfMemoryError("oom message");
});
runtime.overrideCommands(ImmutableList.of(crashCommand));
- // Mocked bug reporter to avoid hard crash.
- BlazeCommandDispatcher dispatch = new BlazeCommandDispatcher(runtime, mock(BugReporter.class));
+ BlazeCommandDispatcher dispatch =
+ new BlazeCommandDispatcher(runtime, BugReporter.defaultInstance());
BlazeCommandResult directResult =
dispatch.exec(ImmutableList.of("testcommand"), "clientdesc", outErr);
diff --git a/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java b/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java
index bd14bfc..6133e37 100644
--- a/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java
+++ b/src/test/java/com/google/devtools/build/lib/runtime/RetainedHeapLimiterTest.java
@@ -40,7 +40,6 @@
import java.lang.management.GarbageCollectorMXBean;
import java.lang.management.MemoryUsage;
import java.lang.ref.WeakReference;
-import java.util.List;
import javax.management.ListenerNotFoundException;
import javax.management.Notification;
import javax.management.NotificationEmitter;
@@ -136,38 +135,21 @@
@Test
public void overThreshold_oom() throws Exception {
- class OomThrowingBugReporter implements BugReporter {
- @Override
- public void sendBugReport(Throwable exception) {
- throw new IllegalStateException(exception);
- }
-
- @Override
- public void sendBugReport(Throwable exception, List<String> args, String... values) {
- throw new IllegalStateException(exception);
- }
-
- @Override
- public RuntimeException handleCrash(Throwable exception, String... args) {
- assertThat(exception).isInstanceOf(OutOfMemoryError.class);
- throw (OutOfMemoryError) exception;
- }
- }
RetainedHeapLimiter underTest =
RetainedHeapLimiter.createFromBeans(
- ImmutableList.of(mockBean), new OomThrowingBugReporter());
+ ImmutableList.of(mockBean), BugReporter.defaultInstance());
underTest.update(90, "Build fewer targets!", events);
+ assertThrows(
+ SecurityException.class, // From attempt to halt jvm in test.
+ () -> underTest.handleNotification(percentUsedAfterForcedGc(91), null));
OutOfMemoryError oom =
- assertThrows(
- OutOfMemoryError.class,
- () -> underTest.handleNotification(percentUsedAfterForcedGc(91), null));
+ assertThrows(OutOfMemoryError.class, BugReport::maybePropagateUnprocessedThrowableIfInTest);
- String expectedOomMessage = BugReport.constructOomExitMessage("Build fewer targets!");
- assertThat(oom).hasMessageThat().contains(expectedOomMessage);
assertThat(oom).hasMessageThat().contains("forcing exit due to GC thrashing");
assertThat(oom).hasMessageThat().contains("tenured space is more than 90% occupied");
- assertContainsEvent(events, expectedOomMessage, EventKind.ERROR);
+ assertContainsEvent(
+ events, BugReport.constructOomExitMessage("Build fewer targets!"), EventKind.FATAL);
}
@Test
