Skip to content

Commit

Permalink
Migrate off Throwables.propagate, and stop rethrowing exceptions fr…
Browse files Browse the repository at this point in the history
…om other threads.

Plus, simplify `CacheLoader` setup slightly.

Fixes #7434

RELNOTES=n/a
PiperOrigin-RevId: 686728158
  • Loading branch information
cpovirk authored and Google Java Core Libraries committed Oct 17, 2024
1 parent ca12c33 commit 8f747d0
Show file tree
Hide file tree
Showing 6 changed files with 34 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package com.google.common.eventbus;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.UncheckedExecutionException;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -281,7 +283,10 @@ class SubscribesToPrimitive {
@Subscribe
public void toInt(int i) {}
}
assertThrows(IllegalArgumentException.class, () -> bus.register(new SubscribesToPrimitive()));
UncheckedExecutionException thrown =
assertThrows(
UncheckedExecutionException.class, () -> bus.register(new SubscribesToPrimitive()));
assertThat(thrown).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
}

/** Records thrown exception information. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
Expand All @@ -34,7 +32,6 @@
import com.google.common.collect.Multimap;
import com.google.common.primitives.Primitives;
import com.google.common.reflect.TypeToken;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.j2objc.annotations.Weak;
import java.lang.reflect.Method;
import java.util.Arrays;
Expand Down Expand Up @@ -150,13 +147,7 @@ Iterator<Subscriber> getSubscribers(Object event) {
private static final LoadingCache<Class<?>, ImmutableList<Method>> subscriberMethodsCache =
CacheBuilder.newBuilder()
.weakKeys()
.build(
new CacheLoader<Class<?>, ImmutableList<Method>>() {
@Override
public ImmutableList<Method> load(Class<?> concreteClass) throws Exception {
return getAnnotatedMethodsNotCached(concreteClass);
}
});
.build(CacheLoader.from(SubscriberRegistry::getAnnotatedMethodsNotCached));

/**
* Returns all subscribers for the given listener grouped by the type of event they subscribe to.
Expand All @@ -173,12 +164,7 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
}

private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
try {
return subscriberMethodsCache.getUnchecked(clazz);
} catch (UncheckedExecutionException e) {
throwIfUnchecked(e.getCause());
throw e;
}
return subscriberMethodsCache.getUnchecked(clazz);
}

private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {
Expand Down Expand Up @@ -220,27 +206,17 @@ private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz
CacheBuilder.newBuilder()
.weakKeys()
.build(
new CacheLoader<Class<?>, ImmutableSet<Class<?>>>() {
// <Class<?>> is actually needed to compile
@SuppressWarnings("RedundantTypeArguments")
@Override
public ImmutableSet<Class<?>> load(Class<?> concreteClass) {
return ImmutableSet.<Class<?>>copyOf(
TypeToken.of(concreteClass).getTypes().rawTypes());
}
});
CacheLoader.from(
concreteClass ->
ImmutableSet.copyOf(TypeToken.of(concreteClass).getTypes().rawTypes())));

/**
* Flattens a class's type hierarchy into a set of {@code Class} objects including all
* superclasses (transitively) and all interfaces implemented by these superclasses.
*/
@VisibleForTesting
static ImmutableSet<Class<?>> flattenHierarchy(Class<?> concreteClass) {
try {
return flattenHierarchyCache.getUnchecked(concreteClass);
} catch (UncheckedExecutionException e) {
throw Throwables.propagate(e.getCause());
}
return flattenHierarchyCache.getUnchecked(concreteClass);
}

private static final class MethodIdentifier {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,20 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;
import static java.util.Objects.requireNonNull;

import com.google.common.annotations.GwtCompatible;
import com.google.common.annotations.GwtIncompatible;
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier;
import com.google.common.base.Throwables;
import com.google.common.collect.Lists;
import com.google.common.collect.Queues;
import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.UndeclaredThrowableException;
import java.util.Collection;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -730,7 +731,9 @@ public static ThreadFactory platformThreadFactory() {
} catch (IllegalAccessException | ClassNotFoundException | NoSuchMethodException e) {
throw new RuntimeException("Couldn't invoke ThreadManager.currentRequestThreadFactory", e);
} catch (InvocationTargetException e) {
throw Throwables.propagate(e.getCause());
throwIfUnchecked(e.getCause());
// This should be impossible: `currentRequestThreadFactory` has no `throws` clause.
throw new UndeclaredThrowableException(e.getCause());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,12 @@

package com.google.common.eventbus;

import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.UncheckedExecutionException;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
Expand Down Expand Up @@ -281,7 +283,10 @@ class SubscribesToPrimitive {
@Subscribe
public void toInt(int i) {}
}
assertThrows(IllegalArgumentException.class, () -> bus.register(new SubscribesToPrimitive()));
UncheckedExecutionException thrown =
assertThrows(
UncheckedExecutionException.class, () -> bus.register(new SubscribesToPrimitive()));
assertThat(thrown).hasCauseThat().isInstanceOf(IllegalArgumentException.class);
}

/** Records thrown exception information. */
Expand Down
36 changes: 6 additions & 30 deletions guava/src/com/google/common/eventbus/SubscriberRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,10 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.MoreObjects;
import com.google.common.base.Objects;
import com.google.common.base.Throwables;
import com.google.common.cache.CacheBuilder;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
Expand All @@ -34,7 +32,6 @@
import com.google.common.collect.Multimap;
import com.google.common.primitives.Primitives;
import com.google.common.reflect.TypeToken;
import com.google.common.util.concurrent.UncheckedExecutionException;
import com.google.j2objc.annotations.Weak;
import java.lang.reflect.Method;
import java.util.Arrays;
Expand Down Expand Up @@ -150,13 +147,7 @@ Iterator<Subscriber> getSubscribers(Object event) {
private static final LoadingCache<Class<?>, ImmutableList<Method>> subscriberMethodsCache =
CacheBuilder.newBuilder()
.weakKeys()
.build(
new CacheLoader<Class<?>, ImmutableList<Method>>() {
@Override
public ImmutableList<Method> load(Class<?> concreteClass) throws Exception {
return getAnnotatedMethodsNotCached(concreteClass);
}
});
.build(CacheLoader.from(SubscriberRegistry::getAnnotatedMethodsNotCached));

/**
* Returns all subscribers for the given listener grouped by the type of event they subscribe to.
Expand All @@ -173,12 +164,7 @@ private Multimap<Class<?>, Subscriber> findAllSubscribers(Object listener) {
}

private static ImmutableList<Method> getAnnotatedMethods(Class<?> clazz) {
try {
return subscriberMethodsCache.getUnchecked(clazz);
} catch (UncheckedExecutionException e) {
throwIfUnchecked(e.getCause());
throw e;
}
return subscriberMethodsCache.getUnchecked(clazz);
}

private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz) {
Expand Down Expand Up @@ -220,27 +206,17 @@ private static ImmutableList<Method> getAnnotatedMethodsNotCached(Class<?> clazz
CacheBuilder.newBuilder()
.weakKeys()
.build(
new CacheLoader<Class<?>, ImmutableSet<Class<?>>>() {
// <Class<?>> is actually needed to compile
@SuppressWarnings("RedundantTypeArguments")
@Override
public ImmutableSet<Class<?>> load(Class<?> concreteClass) {
return ImmutableSet.<Class<?>>copyOf(
TypeToken.of(concreteClass).getTypes().rawTypes());
}
});
CacheLoader.from(
concreteClass ->
ImmutableSet.copyOf(TypeToken.of(concreteClass).getTypes().rawTypes())));

/**
* Flattens a class's type hierarchy into a set of {@code Class} objects including all
* superclasses (transitively) and all interfaces implemented by these superclasses.
*/
@VisibleForTesting
static ImmutableSet<Class<?>> flattenHierarchy(Class<?> concreteClass) {
try {
return flattenHierarchyCache.getUnchecked(concreteClass);
} catch (UncheckedExecutionException e) {
throw Throwables.propagate(e.getCause());
}
return flattenHierarchyCache.getUnchecked(concreteClass);
}

private static final class MethodIdentifier {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;
import static com.google.common.util.concurrent.Internal.toNanosSaturated;
import static java.util.Objects.requireNonNull;

Expand All @@ -24,12 +25,12 @@
import com.google.common.annotations.J2ktIncompatible;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Supplier;
import com.google.common.base.Throwables;
import com.google.common.collect.Lists;
import com.google.common.collect.Queues;
import com.google.common.util.concurrent.ForwardingListenableFuture.SimpleForwardingListenableFuture;
import com.google.errorprone.annotations.CanIgnoreReturnValue;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.UndeclaredThrowableException;
import java.time.Duration;
import java.util.Collection;
import java.util.Iterator;
Expand Down Expand Up @@ -808,7 +809,9 @@ public static ThreadFactory platformThreadFactory() {
} catch (IllegalAccessException | ClassNotFoundException | NoSuchMethodException e) {
throw new RuntimeException("Couldn't invoke ThreadManager.currentRequestThreadFactory", e);
} catch (InvocationTargetException e) {
throw Throwables.propagate(e.getCause());
throwIfUnchecked(e.getCause());
// This should be impossible: `currentRequestThreadFactory` has no `throws` clause.
throw new UndeclaredThrowableException(e.getCause());
}
}

Expand Down

0 comments on commit 8f747d0

Please sign in to comment.