From 61d3f25b1d15247aaf4cc1d7befb92978f1817e9 Mon Sep 17 00:00:00 2001 From: cpovirk Date: Tue, 7 May 2024 05:34:34 -0700 Subject: [PATCH] Use `Throwable.addSuppressed` in the Android copy of `ServiceManager`. It was [added in API Level 19](https://developer.android.com/reference/java/lang/Throwable#addSuppressed(java.lang.Throwable)), so we can rely on it. Also, address an `IdentityHashMapUsage` warning, and migrate off `newIdentityHashMap`. RELNOTES=n/a PiperOrigin-RevId: 631389682 --- .../util/concurrent/ServiceManager.java | 22 +++++++++++++++++-- .../util/concurrent/ServiceManager.java | 4 ++-- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/android/guava/src/com/google/common/util/concurrent/ServiceManager.java b/android/guava/src/com/google/common/util/concurrent/ServiceManager.java index 38727f504247..190c6470e51b 100644 --- a/android/guava/src/com/google/common/util/concurrent/ServiceManager.java +++ b/android/guava/src/com/google/common/util/concurrent/ServiceManager.java @@ -55,8 +55,8 @@ import java.lang.ref.WeakReference; import java.util.Collections; import java.util.EnumSet; +import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -409,7 +409,7 @@ private static final class ServiceManagerState { final Multiset states = servicesByState.keys(); @GuardedBy("monitor") - final Map startupTimers = Maps.newIdentityHashMap(); + final IdentityHashMap startupTimers = new IdentityHashMap<>(); /** * These two booleans are used to mark the state as ready to start. @@ -725,6 +725,9 @@ void checkHealthy() { new IllegalStateException( "Expected to be healthy after starting. The following services are not running: " + Multimaps.filterKeys(servicesByState, not(equalTo(RUNNING)))); + for (Service service : servicesByState.get(State.FAILED)) { + exception.addSuppressed(new FailedService(service)); + } throw exception; } } @@ -796,6 +799,11 @@ public void failed(State from, Throwable failure) { // Log before the transition, so that if the process exits in response to server failure, // there is a higher likelihood that the cause will be in the logs. boolean log = !(service instanceof NoOpService); + /* + * We have already exposed startup exceptions to the user in the form of suppressed + * exceptions. We don't need to log those exceptions again. + */ + log &= from != State.STARTING; if (log) { logger .get() @@ -831,4 +839,14 @@ protected void doStop() { /** This is never thrown but only used for logging. */ private static final class EmptyServiceManagerWarning extends Throwable {} + + private static final class FailedService extends Throwable { + FailedService(Service service) { + super( + service.toString(), + service.failureCause(), + false /* don't enable suppression */, + false /* don't calculate a stack trace. */); + } + } } diff --git a/guava/src/com/google/common/util/concurrent/ServiceManager.java b/guava/src/com/google/common/util/concurrent/ServiceManager.java index 0bda0afecc4b..1f11a6b252ff 100644 --- a/guava/src/com/google/common/util/concurrent/ServiceManager.java +++ b/guava/src/com/google/common/util/concurrent/ServiceManager.java @@ -58,8 +58,8 @@ import java.time.Duration; import java.util.Collections; import java.util.EnumSet; +import java.util.IdentityHashMap; import java.util.List; -import java.util.Map; import java.util.Map.Entry; import java.util.concurrent.Executor; import java.util.concurrent.TimeUnit; @@ -454,7 +454,7 @@ private static final class ServiceManagerState { final Multiset states = servicesByState.keys(); @GuardedBy("monitor") - final Map startupTimers = Maps.newIdentityHashMap(); + final IdentityHashMap startupTimers = new IdentityHashMap<>(); /** * These two booleans are used to mark the state as ready to start.