RELNOTES[INC]: --fatal_event_bus_exceptions is deprecated and should not be used. Any crashes should be reported so that they can be fixed.
Allow Blaze modules to inject a SubscriberExceptionHandler/uncaught exception handler. The provided handler will be used to handle EventBus exceptions as well as any crashes in async threads.
PiperOrigin-RevId: 296124771
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 d4cebc7..f79a554 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
@@ -165,11 +165,11 @@
*
* <p>Has no effect if another crash has already been handled by {@link BugReport}.
*/
- public static void handleCrash(Throwable throwable, String... args) {
- handleCrash(throwable, /*sendBugReport=*/ true, /*exitCode=*/ null, args);
+ public static RuntimeException handleCrash(Throwable throwable, String... args) {
+ throw handleCrash(throwable, /*sendBugReport=*/ true, /*exitCode=*/ null, args);
}
- private static void handleCrash(
+ private static RuntimeException handleCrash(
Throwable throwable, boolean sendBugReport, @Nullable ExitCode exitCode, String... args) {
ExitCode exitCodeToUse = exitCode == null ? getExitCodeForThrowable(throwable) : exitCode;
int numericExitCode = exitCodeToUse.getNumericExitCode();
@@ -178,7 +178,10 @@
if (IN_TEST) {
unprocessedThrowableInTest = throwable;
}
- logCrash(throwable, sendBugReport, args);
+ // Don't try to send a bug report during a crash in a test, it will throw itself.
+ if (!IN_TEST || !sendBugReport) {
+ logCrash(throwable, sendBugReport, args);
+ }
try {
if (runtime != null) {
runtime.cleanUpForCrash(exitCodeToUse);
@@ -209,6 +212,7 @@
Runtime.getRuntime().halt(numericExitCode);
}
+ throw new IllegalStateException("never get here", throwable);
}
/** Get exit code corresponding to throwable. */
@@ -292,5 +296,10 @@
public void sendBugReport(Throwable exception, List<String> args, String... values) {
BugReport.sendBugReport(exception, args, values);
}
+
+ @Override
+ public RuntimeException handleCrash(Throwable throwable, String... args) {
+ throw BugReport.handleCrash(throwable, args);
+ }
}
}
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 57dca7d..3fc8cea 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
@@ -33,4 +33,7 @@
/** Reports an exception, see {@link BugReport#sendBugReport(Throwable, List, String[])}. */
void sendBugReport(Throwable exception, List<String> args, String... values);
+
+ /** See {@link BugReport#handleCrash}. */
+ RuntimeException handleCrash(Throwable throwable, String... args);
}
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
index 8ec26a8..1c6d635 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeModule.java
@@ -16,6 +16,8 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
+import com.google.common.eventbus.SubscriberExceptionContext;
+import com.google.common.eventbus.SubscriberExceptionHandler;
import com.google.devtools.build.lib.actions.ExecutorInitException;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.BlazeVersionInfo;
@@ -125,6 +127,18 @@
}
/**
+ * Returns handler for {@link com.google.common.eventbus.EventBus} subscriber and async thread
+ * exceptions. For async thread exceptions, {@link
+ * SubscriberExceptionHandler#handleException(Throwable, SubscriberExceptionContext)} will be
+ * called with null {@link SubscriberExceptionContext}. If all modules return null, a handler that
+ * crashes on all async exceptions and files bug reports for all EventBus subscriber exceptions
+ * will be used.
+ */
+ public SubscriberExceptionHandler getEventBusAndAsyncExceptionHandler() {
+ return null;
+ }
+
+ /**
* Called when Bazel starts up after {@link #getStartupOptions}, {@link #globalInit}, and {@link
* #getFileSystem}.
*
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 44bec37..fe5e679 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
@@ -21,7 +21,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.eventbus.EventBus;
-import com.google.common.eventbus.SubscriberExceptionContext;
import com.google.common.eventbus.SubscriberExceptionHandler;
import com.google.common.util.concurrent.Futures;
import com.google.common.util.concurrent.Uninterruptibles;
@@ -531,14 +530,11 @@
}
/**
- * Hook method called by the BlazeCommandDispatcher prior to the dispatch of
- * each command.
+ * Hook method called by the BlazeCommandDispatcher prior to the dispatch of each command.
*
* @param options The CommonCommandOptions used by every command.
- * @throws AbruptExitException if this command is unsuitable to be run as specified
*/
- void beforeCommand(CommandEnvironment env, CommonCommandOptions options)
- throws AbruptExitException {
+ void beforeCommand(CommandEnvironment env, CommonCommandOptions options) {
if (options.memoryProfilePath != null) {
Path memoryProfilePath = env.getWorkingDirectory().getRelative(options.memoryProfilePath);
MemoryProfiler.instance()
@@ -798,36 +794,11 @@
}
/**
- * An EventBus exception handler that will report the exception to a remote server, if a
- * handler is registered.
- */
- public static final class RemoteExceptionHandler implements SubscriberExceptionHandler {
- static final String FAILURE_MSG = "Failure in EventBus subscriber";
-
- @Override
- public void handleException(Throwable exception, SubscriberExceptionContext context) {
- logger.log(Level.SEVERE, FAILURE_MSG, exception);
- LoggingUtil.logToRemote(Level.SEVERE, FAILURE_MSG, exception);
- }
- }
-
- /**
- * An EventBus exception handler that will call BugReport.handleCrash exiting
- * the current thread.
- */
- public static final class BugReportingExceptionHandler implements SubscriberExceptionHandler {
- @Override
- public void handleException(Throwable exception, SubscriberExceptionContext context) {
- BugReport.handleCrash(exception);
- }
- }
-
- /**
* Main method for the Blaze server startup. Note: This method logs
* exceptions to remote servers. Do not add this to a unittest.
*/
public static void main(Iterable<Class<? extends BlazeModule>> moduleClasses, String[] args) {
- setupUncaughtHandler(args);
+ setupUncaughtHandlerAtStartup(args);
List<BlazeModule> modules = createModules(moduleClasses);
// blaze.cc will put --batch first if the user set it.
if (args.length >= 1 && args[0].equals("--batch")) {
@@ -1275,6 +1246,34 @@
throw new AbruptExitException(
"No module set the default hash function.", ExitCode.BLAZE_INTERNAL_ERROR, e);
}
+
+ SubscriberExceptionHandler currentHandlerValue = null;
+ for (BlazeModule module : blazeModules) {
+ SubscriberExceptionHandler newHandler = module.getEventBusAndAsyncExceptionHandler();
+ if (newHandler != null) {
+ Preconditions.checkState(
+ currentHandlerValue == null, "Two handlers given. Last module: %s", module);
+ currentHandlerValue = newHandler;
+ }
+ }
+ if (currentHandlerValue == null) {
+ if (startupOptions.fatalEventBusExceptions) {
+ currentHandlerValue = (exception, context) -> BugReport.handleCrash(exception);
+ } else {
+ currentHandlerValue =
+ (exception, context) -> {
+ if (context == null) {
+ BugReport.handleCrash(exception);
+ } else {
+ BugReport.sendBugReport(
+ exception, ImmutableList.of(), "Failure in EventBus subscriber");
+ }
+ };
+ }
+ }
+ SubscriberExceptionHandler subscriberExceptionHandler = currentHandlerValue;
+ Thread.setDefaultUncaughtExceptionHandler(
+ (thread, throwable) -> subscriberExceptionHandler.handleException(throwable, null));
Path.setFileSystemForSerialization(fs);
SubprocessBuilder.setDefaultSubprocessFactory(subprocessFactoryImplementation());
@@ -1310,13 +1309,7 @@
.setStartupOptionsProvider(options)
.setClock(clock)
.setAbruptShutdownHandler(abruptShutdownHandler)
- // TODO(bazel-team): Make BugReportingExceptionHandler the default.
- // See bug "Make exceptions in EventBus subscribers fatal"
- .setEventBusExceptionHandler(
- startupOptions.fatalEventBusExceptions
- || !BlazeVersionInfo.instance().isReleasedBlaze()
- ? new BlazeRuntime.BugReportingExceptionHandler()
- : new BlazeRuntime.RemoteExceptionHandler());
+ .setEventBusExceptionHandler(subscriberExceptionHandler);
if (System.getenv("TEST_TMPDIR") != null
&& System.getenv("NO_CRASH_ON_LOGGING_IN_TEST") == null) {
@@ -1426,10 +1419,10 @@
}
/**
- * Make sure async threads cannot be orphaned. This method makes sure bugs are reported to
- * telemetry and the proper exit code is reported.
+ * Make sure async threads cannot be orphaned at startup. This method makes sure bugs are reported
+ * to telemetry and the proper exit code is reported. Will be overwritten with better handler.
*/
- private static void setupUncaughtHandler(final String[] args) {
+ private static void setupUncaughtHandlerAtStartup(final String[] args) {
Thread.setDefaultUncaughtExceptionHandler(
(thread, throwable) -> BugReport.handleCrash(throwable, args));
}
@@ -1456,8 +1449,8 @@
* BlazeDirectories}, and the {@link RuleClassProvider} (except for testing). All other fields
* have safe default values.
*
- * <p>The default behavior of the BlazeRuntime's EventBus is to exit when a subscriber throws
- * an exception. Please plan appropriately.
+ * <p>The default behavior of the BlazeRuntime's EventBus is to exit the JVM when a subscriber
+ * throws an exception. Please plan appropriately.
*/
public static class Builder {
private FileSystem fileSystem;
@@ -1466,7 +1459,8 @@
private Runnable abruptShutdownHandler;
private OptionsParsingResult startupOptionsProvider;
private final List<BlazeModule> blazeModules = new ArrayList<>();
- private SubscriberExceptionHandler eventBusExceptionHandler = new RemoteExceptionHandler();
+ private SubscriberExceptionHandler eventBusExceptionHandler =
+ (throwable, context) -> BugReport.handleCrash(throwable);
private UUID instanceId;
private String productName;
private ActionKeyContext actionKeyContext;
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
index 16ccd28..b8c65e9 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeServerStartupOptions.java
@@ -308,12 +308,12 @@
public boolean ignoreAllRcFiles;
@Option(
- name = "fatal_event_bus_exceptions",
- defaultValue = "false", // NOTE: only for documentation, value is always passed by the client.
- documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
- effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT, OptionEffectTag.LOSES_INCREMENTAL_STATE},
- help = "Whether or not to exit if an exception is thrown by an internal EventBus handler."
- )
+ name = "fatal_event_bus_exceptions",
+ defaultValue = "false", // NOTE: only for documentation, value is always passed by the client.
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+ effectTags = {OptionEffectTag.EAGERNESS_TO_EXIT, OptionEffectTag.LOSES_INCREMENTAL_STATE},
+ deprecationWarning = "Will be enabled by default and removed soon",
+ help = "Whether or not to exit if an exception is thrown by an internal EventBus handler.")
public boolean fatalEventBusExceptions;
@Option(
