Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Be clearer about what types we're catching. #6033

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public final void run() {
+ e.getClass()
+ " without a cause");
}
} catch (Throwable e) { // this includes cancellation exception
} catch (RuntimeException | Error e) { // this includes cancellation exception
throwable = e;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.common.util.concurrent;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;
import static com.google.common.util.concurrent.NullnessCasts.uncheckedNull;
import static java.lang.Integer.toHexString;
import static java.lang.System.identityHashCode;
Expand Down Expand Up @@ -159,7 +158,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {

try {
helper = new UnsafeAtomicHelper();
} catch (Throwable unsafeFailure) {
} catch (RuntimeException | Error unsafeFailure) {
thrownUnsafeFailure = unsafeFailure;
// catch absolutely everything and fall through to our 'SafeAtomicHelper'
// The access control checks that ARFU does means the caller class has to be AbstractFuture
Expand All @@ -172,7 +171,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
newUpdater(AbstractFuture.class, Object.class, "value"));
} catch (Throwable atomicReferenceFieldUpdaterFailure) {
} catch (RuntimeException | Error atomicReferenceFieldUpdaterFailure) {
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
// For these users fallback to a suboptimal implementation, based on synchronized. This will
Expand Down Expand Up @@ -859,14 +858,14 @@ protected boolean setFuture(ListenableFuture<? extends V> future) {
// since all we are doing is unpacking a completed future which should be fast.
try {
future.addListener(valueToSet, DirectExecutor.INSTANCE);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
// addListener has thrown an exception! SetFuture.run can't throw any exceptions so this
// must have been caused by addListener itself. The most likely explanation is a
// misconfigured mock. Try to switch to Failure.
Failure failure;
try {
failure = new Failure(t);
} catch (Throwable oomMostLikely) {
} catch (RuntimeException | Error oomMostLikely) {
failure = Failure.FALLBACK_INSTANCE;
}
// Note: The only way this CAS could fail is if cancel() has raced with us. That is ok.
Expand Down Expand Up @@ -961,7 +960,7 @@ private static Object getFutureValue(ListenableFuture<?> future) {
cancellation));
}
return new Cancellation(false, cancellation);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
return new Failure(t);
}
}
Expand Down Expand Up @@ -1353,9 +1352,10 @@ public sun.misc.Unsafe run() throws Exception {
WAITER_THREAD_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("thread"));
WAITER_NEXT_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("next"));
UNSAFE = unsafe;
} catch (Exception e) {
throwIfUnchecked(e);
} catch (NoSuchFieldException e) {
throw new RuntimeException(e);
} catch (RuntimeException e) {
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,7 @@ public Cancellable reschedule() {
lock.lock();
try {
toReturn = initializeOrUpdateCancellationDelegate(schedule);
} catch (Throwable e) {
} catch (RuntimeException | Error e) {
// If an exception is thrown by the subclass then we need to make sure that the service
// notices and transitions to the FAILED state. We do it by calling notifyFailed directly
// because the service does not monitor the state of the future so if the exception is not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private void collectValueFromNonCancelledFuture(int index, Future<? extends Inpu
collectOneValue(index, getDone(future));
} catch (ExecutionException e) {
handleException(e.getCause());
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
handleException(t);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>
new SafeAtomicHelper(
newUpdater(AggregateFutureState.class, Set.class, "seenExceptions"),
newUpdater(AggregateFutureState.class, "remaining"));
} catch (Throwable reflectionFailure) {
} catch (RuntimeException | Error reflectionFailure) {
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
// For these users fallback to a suboptimal implementation, based on synchronized. This will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ public <T> T newProxy(
throw new ExecutionException(e);
} catch (Error e) {
throw new ExecutionError(e);
} catch (Throwable e) {
// It's a non-Error, non-Exception Throwable. Such classes are usually intended to extend
// Exception, so we'll treat it like an Exception.
throw new ExecutionException(e);
}
}

Expand All @@ -86,10 +82,6 @@ public void runWithTimeout(Runnable runnable, long timeoutDuration, TimeUnit tim
throw new UncheckedExecutionException(e);
} catch (Error e) {
throw new ExecutionError(e);
} catch (Throwable e) {
// It's a non-Error, non-Exception Throwable. Such classes are usually intended to extend
// Exception, so we'll treat it like a RuntimeException.
throw new UncheckedExecutionException(e);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ public O get(long timeout, TimeUnit unit)
private O applyTransformation(I input) throws ExecutionException {
try {
return function.apply(input);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
throw new ExecutionException(t);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ private static boolean hasConstructorUsableByGetChecked(
try {
Exception unused = newWithCause(exceptionClass, new Exception());
return true;
} catch (Exception e) {
} catch (RuntimeException | Error e) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.annotations.Beta;
import com.google.common.annotations.GwtIncompatible;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
Expand Down Expand Up @@ -158,9 +159,10 @@ public void addListener(Runnable listener, Executor exec) {
* to return a proper ListenableFuture instead of using listenInPoolThread.
*/
getUninterruptibly(delegate);
} catch (Throwable e) {
// ExecutionException / CancellationException / RuntimeException / Error
} catch (ExecutionException | RuntimeException | Error e) {
// (including CancellationException)
// The task is presumably done, run the listeners.
// TODO(cpovirk): Do *something* in case of Error (and maybe RuntimeException)?
}
executionList.execute();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1011,7 +1011,7 @@ private void signalNextWaiter() {
private boolean isSatisfied(Guard guard) {
try {
return guard.isSatisfied();
} catch (Throwable throwable) {
} catch (RuntimeException | Error throwable) {
signalAllWaiters();
throw throwable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -667,9 +667,9 @@ public NeverSuccessfulListenableFutureTask(Runnable delegate) {
public void run() {
try {
delegate.run();
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
setException(t);
throw Throwables.propagate(t);
throw t;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void uncaughtException(Thread t, Throwable e) {
try {
logger.log(
SEVERE, String.format(Locale.ROOT, "Caught an exception in %s. Shutting down.", t), e);
} catch (Throwable errorInLogging) {
} catch (RuntimeException | Error errorInLogging) {
// If logging fails, e.g. due to missing memory, at least try to log the
// message and the cause for the failed logging.
System.err.println(e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public final void run() {
+ e.getClass()
+ " without a cause");
}
} catch (Throwable e) { // this includes cancellation exception
} catch (RuntimeException | Error e) { // this includes cancellation exception
throwable = e;
}

Expand Down
16 changes: 8 additions & 8 deletions guava/src/com/google/common/util/concurrent/AbstractFuture.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
package com.google.common.util.concurrent;

import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Throwables.throwIfUnchecked;
import static com.google.common.util.concurrent.NullnessCasts.uncheckedNull;
import static java.lang.Integer.toHexString;
import static java.lang.System.identityHashCode;
Expand Down Expand Up @@ -159,7 +158,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {

try {
helper = new UnsafeAtomicHelper();
} catch (Throwable unsafeFailure) {
} catch (RuntimeException | Error unsafeFailure) {
thrownUnsafeFailure = unsafeFailure;
// catch absolutely everything and fall through to our 'SafeAtomicHelper'
// The access control checks that ARFU does means the caller class has to be AbstractFuture
Expand All @@ -172,7 +171,7 @@ public final boolean cancel(boolean mayInterruptIfRunning) {
newUpdater(AbstractFuture.class, Waiter.class, "waiters"),
newUpdater(AbstractFuture.class, Listener.class, "listeners"),
newUpdater(AbstractFuture.class, Object.class, "value"));
} catch (Throwable atomicReferenceFieldUpdaterFailure) {
} catch (RuntimeException | Error atomicReferenceFieldUpdaterFailure) {
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
// For these users fallback to a suboptimal implementation, based on synchronized. This will
Expand Down Expand Up @@ -859,14 +858,14 @@ protected boolean setFuture(ListenableFuture<? extends V> future) {
// since all we are doing is unpacking a completed future which should be fast.
try {
future.addListener(valueToSet, DirectExecutor.INSTANCE);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
// addListener has thrown an exception! SetFuture.run can't throw any exceptions so this
// must have been caused by addListener itself. The most likely explanation is a
// misconfigured mock. Try to switch to Failure.
Failure failure;
try {
failure = new Failure(t);
} catch (Throwable oomMostLikely) {
} catch (RuntimeException | Error oomMostLikely) {
failure = Failure.FALLBACK_INSTANCE;
}
// Note: The only way this CAS could fail is if cancel() has raced with us. That is ok.
Expand Down Expand Up @@ -961,7 +960,7 @@ private static Object getFutureValue(ListenableFuture<?> future) {
cancellation));
}
return new Cancellation(false, cancellation);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
return new Failure(t);
}
}
Expand Down Expand Up @@ -1353,9 +1352,10 @@ public sun.misc.Unsafe run() throws Exception {
WAITER_THREAD_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("thread"));
WAITER_NEXT_OFFSET = unsafe.objectFieldOffset(Waiter.class.getDeclaredField("next"));
UNSAFE = unsafe;
} catch (Exception e) {
throwIfUnchecked(e);
} catch (NoSuchFieldException e) {
throw new RuntimeException(e);
} catch (RuntimeException e) {
throw e;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ public Cancellable reschedule() {
lock.lock();
try {
toReturn = initializeOrUpdateCancellationDelegate(schedule);
} catch (Throwable e) {
} catch (RuntimeException | Error e) {
// If an exception is thrown by the subclass then we need to make sure that the service
// notices and transitions to the FAILED state. We do it by calling notifyFailed directly
// because the service does not monitor the state of the future so if the exception is not
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ private void collectValueFromNonCancelledFuture(int index, Future<? extends Inpu
collectOneValue(index, getDone(future));
} catch (ExecutionException e) {
handleException(e.getCause());
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
handleException(t);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ abstract class AggregateFutureState<OutputT extends @Nullable Object>
new SafeAtomicHelper(
newUpdater(AggregateFutureState.class, Set.class, "seenExceptions"),
newUpdater(AggregateFutureState.class, "remaining"));
} catch (Throwable reflectionFailure) {
} catch (RuntimeException | Error reflectionFailure) {
// Some Android 5.0.x Samsung devices have bugs in JDK reflection APIs that cause
// getDeclaredField to throw a NoSuchFieldException when the field is definitely there.
// For these users fallback to a suboptimal implementation, based on synchronized. This will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,6 @@ public <T> T newProxy(
throw new ExecutionException(e);
} catch (Error e) {
throw new ExecutionError(e);
} catch (Throwable e) {
// It's a non-Error, non-Exception Throwable. Such classes are usually intended to extend
// Exception, so we'll treat it like an Exception.
throw new ExecutionException(e);
}
}

Expand All @@ -86,10 +82,6 @@ public void runWithTimeout(Runnable runnable, long timeoutDuration, TimeUnit tim
throw new UncheckedExecutionException(e);
} catch (Error e) {
throw new ExecutionError(e);
} catch (Throwable e) {
// It's a non-Error, non-Exception Throwable. Such classes are usually intended to extend
// Exception, so we'll treat it like a RuntimeException.
throw new UncheckedExecutionException(e);
}
}

Expand Down
2 changes: 1 addition & 1 deletion guava/src/com/google/common/util/concurrent/Futures.java
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,7 @@ public O get(long timeout, TimeUnit unit)
private O applyTransformation(I input) throws ExecutionException {
try {
return function.apply(input);
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
throw new ExecutionException(t);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,9 @@ static GetCheckedTypeValidator getBestValidator() {
Class<? extends Enum> theClass =
Class.forName(CLASS_VALUE_VALIDATOR_NAME).asSubclass(Enum.class);
return (GetCheckedTypeValidator) theClass.getEnumConstants()[0];
} catch (Throwable t) { // ensure we really catch *everything*
} catch (ClassNotFoundException
| RuntimeException
| Error t) { // ensure we really catch *everything*
return weakSetValidator();
}
}
Expand Down Expand Up @@ -221,7 +223,7 @@ private static boolean hasConstructorUsableByGetChecked(
try {
Exception unused = newWithCause(exceptionClass, new Exception());
return true;
} catch (Exception e) {
} catch (RuntimeException | Error e) {
return false;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.annotations.Beta;
import com.google.common.annotations.GwtIncompatible;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
Expand Down Expand Up @@ -158,9 +159,10 @@ public void addListener(Runnable listener, Executor exec) {
* to return a proper ListenableFuture instead of using listenInPoolThread.
*/
getUninterruptibly(delegate);
} catch (Throwable e) {
// ExecutionException / CancellationException / RuntimeException / Error
} catch (ExecutionException | RuntimeException | Error e) {
// (including CancellationException)
// The task is presumably done, run the listeners.
// TODO(cpovirk): Do *something* in case of Error (and maybe RuntimeException)?
}
executionList.execute();
});
Expand Down
2 changes: 1 addition & 1 deletion guava/src/com/google/common/util/concurrent/Monitor.java
Original file line number Diff line number Diff line change
Expand Up @@ -1121,7 +1121,7 @@ private void signalNextWaiter() {
private boolean isSatisfied(Guard guard) {
try {
return guard.isSatisfied();
} catch (Throwable throwable) {
} catch (RuntimeException | Error throwable) {
signalAllWaiters();
throw throwable;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -728,9 +728,9 @@ public NeverSuccessfulListenableFutureTask(Runnable delegate) {
public void run() {
try {
delegate.run();
} catch (Throwable t) {
} catch (RuntimeException | Error t) {
setException(t);
throw Throwables.propagate(t);
throw t;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void uncaughtException(Thread t, Throwable e) {
try {
logger.log(
SEVERE, String.format(Locale.ROOT, "Caught an exception in %s. Shutting down.", t), e);
} catch (Throwable errorInLogging) {
} catch (RuntimeException | Error errorInLogging) {
// If logging fails, e.g. due to missing memory, at least try to log the
// message and the cause for the failed logging.
System.err.println(e.getMessage());
Expand Down