From fe3201c9f76b3e70107a322e1985a2e79a692b9d Mon Sep 17 00:00:00 2001 From: Matko Medenjak Date: Fri, 11 Sep 2020 13:15:08 +0200 Subject: [PATCH 1/2] Revert "Add caller stacktrace to rethrown RuntimeException (#17249)" This reverts commit 318b336b. Reasoning is that it introduces significant changes in the invocation handling and causes some tests to start failing. An alternate or augmented solution must be considered. --- .../HazelcastClientOfflineException.java | 4 -- .../internal/util/ExceptionUtil.java | 59 +------------------ .../ringbuffer/StaleSequenceException.java | 11 +--- .../spi/impl/AbstractInvocationFuture.java | 52 ++++++++++++++-- ...ClientUserCodeDeploymentExceptionTest.java | 13 +++- .../internal/util/ExceptionUtilTest.java | 6 +- ...ehind_whenNoCoalescingQueueIsFullTest.java | 2 + .../impl/DistributedObjectFutureTest.java | 2 +- 8 files changed, 67 insertions(+), 82 deletions(-) diff --git a/hazelcast/src/main/java/com/hazelcast/client/HazelcastClientOfflineException.java b/hazelcast/src/main/java/com/hazelcast/client/HazelcastClientOfflineException.java index 96d5b66d80c0..f3e2366a08d2 100644 --- a/hazelcast/src/main/java/com/hazelcast/client/HazelcastClientOfflineException.java +++ b/hazelcast/src/main/java/com/hazelcast/client/HazelcastClientOfflineException.java @@ -24,8 +24,4 @@ public class HazelcastClientOfflineException extends IllegalStateException { public HazelcastClientOfflineException() { super("No connection found to cluster"); } - - public HazelcastClientOfflineException(Throwable cause) { - super(cause); - } } diff --git a/hazelcast/src/main/java/com/hazelcast/internal/util/ExceptionUtil.java b/hazelcast/src/main/java/com/hazelcast/internal/util/ExceptionUtil.java index 5d0903b404a4..80eb3fb414e8 100644 --- a/hazelcast/src/main/java/com/hazelcast/internal/util/ExceptionUtil.java +++ b/hazelcast/src/main/java/com/hazelcast/internal/util/ExceptionUtil.java @@ -19,14 +19,10 @@ import com.hazelcast.core.HazelcastException; import com.hazelcast.instance.impl.OutOfMemoryErrorDispatcher; import com.hazelcast.logging.ILogger; -import com.hazelcast.spi.impl.operationservice.WrappableException; import javax.annotation.Nonnull; import java.io.PrintWriter; import java.io.StringWriter; -import java.lang.invoke.MethodHandle; -import java.lang.invoke.MethodHandles; -import java.lang.invoke.MethodType; import java.lang.reflect.InvocationTargetException; import java.util.concurrent.ExecutionException; import java.util.function.BiConsumer; @@ -37,14 +33,6 @@ */ public final class ExceptionUtil { - private static final MethodHandles.Lookup LOOKUP = MethodHandles.publicLookup(); - // new Throwable(String message, Throwable cause) - private static final MethodType MT_INIT_STRING_THROWABLE = MethodType.methodType(void.class, String.class, Throwable.class); - // new Throwable(Throwable cause) - private static final MethodType MT_INIT_THROWABLE = MethodType.methodType(void.class, Throwable.class); - // new Throwable(String message) - private static final MethodType MT_INIT_STRING = MethodType.methodType(void.class, String.class); - private static final BiFunction HAZELCAST_EXCEPTION_WRAPPER = (throwable, message) -> { if (message != null) { return new HazelcastException(message, throwable); @@ -111,7 +99,7 @@ public static Throwable peel(final Throwable t, Class a public static Throwable peel(final Throwable t, Class allowedType, String message, BiFunction exceptionWrapper) { if (t instanceof RuntimeException) { - return wrapException(t, message, exceptionWrapper); + return t; } if (t instanceof ExecutionException || t instanceof InvocationTargetException) { @@ -130,20 +118,7 @@ public static Throwable peel(final Throwable t, Class Throwable wrapException(Throwable t, String message, - BiFunction exceptionWrapper) { - if (t instanceof WrappableException) { - return ((WrappableException) t).wrap(); - } - Throwable wrapped = tryWrapInSameClass(t); - return wrapped == null ? exceptionWrapper.apply(t, message) : wrapped; - } - - public static RuntimeException wrapException(RuntimeException t) { - return (RuntimeException) wrapException(t, null, HAZELCAST_EXCEPTION_WRAPPER); - } - - public static RuntimeException rethrow(final Throwable t) { + public static RuntimeException rethrow(Throwable t) { rethrowIfError(t); throw peel(t); } @@ -176,15 +151,10 @@ public static void rethrowIfError(final Throwable t) { if (t instanceof OutOfMemoryError) { OutOfMemoryErrorDispatcher.onOutOfMemory((OutOfMemoryError) t); } - throw wrapError((Error) t); + throw (Error) t; } } - public static Error wrapError(Error cause) { - Error result = tryWrapInSameClass(cause); - return result == null ? cause : result; - } - public static RuntimeException rethrowAllowInterrupted(final Throwable t) throws InterruptedException { return rethrow(t, InterruptedException.class); } @@ -224,27 +194,4 @@ public static RuntimeException sneakyThrow(@Nonnull Throwa } }; } - - public static T tryWrapInSameClass(T cause) { - Class exceptionClass = cause.getClass(); - MethodHandle constructor; - try { - constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_STRING_THROWABLE); - return (T) constructor.invokeWithArguments(cause.getMessage(), cause); - } catch (Throwable ignored) { - } - try { - constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_THROWABLE); - return (T) constructor.invokeWithArguments(cause); - } catch (Throwable ignored) { - } - try { - constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_STRING); - T result = (T) constructor.invokeWithArguments(cause.getMessage()); - result.initCause(cause); - return result; - } catch (Throwable ignored) { - } - return null; - } } diff --git a/hazelcast/src/main/java/com/hazelcast/ringbuffer/StaleSequenceException.java b/hazelcast/src/main/java/com/hazelcast/ringbuffer/StaleSequenceException.java index 0c2b130ca5e4..2039004fb54f 100644 --- a/hazelcast/src/main/java/com/hazelcast/ringbuffer/StaleSequenceException.java +++ b/hazelcast/src/main/java/com/hazelcast/ringbuffer/StaleSequenceException.java @@ -17,15 +17,13 @@ package com.hazelcast.ringbuffer; import com.hazelcast.spi.exception.SilentException; -import com.hazelcast.spi.impl.operationservice.WrappableException; /** * An {@link RuntimeException} that is thrown when accessing an item in the {@link Ringbuffer} using a sequence that is smaller * than the current head sequence and that the ringbuffer store is disabled. This means that the item isn't available in the * ringbuffer and it cannot be loaded from the store either, thus being completely unavailable. */ -public class StaleSequenceException extends RuntimeException - implements SilentException, WrappableException { +public class StaleSequenceException extends RuntimeException implements SilentException { private final long headSeq; @@ -49,11 +47,4 @@ public StaleSequenceException(String message, long headSeq) { public long getHeadSeq() { return headSeq; } - - @Override - public StaleSequenceException wrap() { - StaleSequenceException staleSequenceException = new StaleSequenceException(getMessage(), headSeq); - staleSequenceException.initCause(this); - return staleSequenceException; - } } diff --git a/hazelcast/src/main/java/com/hazelcast/spi/impl/AbstractInvocationFuture.java b/hazelcast/src/main/java/com/hazelcast/spi/impl/AbstractInvocationFuture.java index 690eb886224d..35e9e77830d0 100644 --- a/hazelcast/src/main/java/com/hazelcast/spi/impl/AbstractInvocationFuture.java +++ b/hazelcast/src/main/java/com/hazelcast/spi/impl/AbstractInvocationFuture.java @@ -22,10 +22,15 @@ import com.hazelcast.instance.impl.OutOfMemoryErrorDispatcher; import com.hazelcast.internal.util.executor.UnblockableThread; import com.hazelcast.logging.ILogger; +import com.hazelcast.spi.impl.operationservice.WrappableException; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import java.lang.invoke.MethodHandles.Lookup; +import java.lang.invoke.MethodType; import java.lang.reflect.InvocationTargetException; import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; @@ -46,8 +51,6 @@ import static com.hazelcast.internal.util.ConcurrencyUtil.DEFAULT_ASYNC_EXECUTOR; import static com.hazelcast.internal.util.ExceptionUtil.sneakyThrow; -import static com.hazelcast.internal.util.ExceptionUtil.wrapError; -import static com.hazelcast.internal.util.ExceptionUtil.wrapException; import static java.util.Objects.requireNonNull; import static java.util.concurrent.atomic.AtomicReferenceFieldUpdater.newUpdater; import static java.util.concurrent.locks.LockSupport.park; @@ -56,7 +59,6 @@ /** * Custom implementation of {@link java.util.concurrent.CompletableFuture}. - * * @param */ @SuppressFBWarnings(value = "DLS_DEAD_STORE_OF_CLASS_LITERAL", justification = "Recommended way to prevent classloading bug") @@ -69,6 +71,13 @@ public String toString() { return "UNRESOLVED"; } }; + private static final Lookup LOOKUP = MethodHandles.publicLookup(); + // new Throwable(String message, Throwable cause) + private static final MethodType MT_INIT_STRING_THROWABLE = MethodType.methodType(void.class, String.class, Throwable.class); + // new Throwable(Throwable cause) + private static final MethodType MT_INIT_THROWABLE = MethodType.methodType(void.class, Throwable.class); + // new Throwable(String message) + private static final MethodType MT_INIT_STRING = MethodType.methodType(void.class, String.class); private static final AtomicReferenceFieldUpdater STATE_UPDATER = newUpdater(AbstractInvocationFuture.class, Object.class, "state"); @@ -1903,7 +1912,7 @@ static Throwable wrapOrPeel(Throwable cause) { return cause; } if (cause instanceof RuntimeException) { - return wrapException((RuntimeException) cause); + return wrapRuntimeException((RuntimeException) cause); } if ((cause instanceof ExecutionException || cause instanceof InvocationTargetException) && cause.getCause() != null) { @@ -1918,4 +1927,39 @@ static Throwable wrapOrPeel(Throwable cause) { return new HazelcastException(cause); } + private static RuntimeException wrapRuntimeException(RuntimeException cause) { + if (cause instanceof WrappableException) { + return ((WrappableException) cause).wrap(); + } + RuntimeException wrapped = tryWrapInSameClass(cause); + return wrapped == null ? new HazelcastException(cause) : wrapped; + } + + private static Error wrapError(Error cause) { + Error result = tryWrapInSameClass(cause); + return result == null ? cause : result; + } + + private static T tryWrapInSameClass(T cause) { + Class exceptionClass = cause.getClass(); + MethodHandle constructor; + try { + constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_STRING_THROWABLE); + return (T) constructor.invokeWithArguments(cause.getMessage(), cause); + } catch (Throwable ignored) { + } + try { + constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_THROWABLE); + return (T) constructor.invokeWithArguments(cause); + } catch (Throwable ignored) { + } + try { + constructor = LOOKUP.findConstructor(exceptionClass, MT_INIT_STRING); + T result = (T) constructor.invokeWithArguments(cause.getMessage()); + result.initCause(cause); + return result; + } catch (Throwable ignored) { + } + return null; + } } diff --git a/hazelcast/src/test/java/com/hazelcast/client/usercodedeployment/ClientUserCodeDeploymentExceptionTest.java b/hazelcast/src/test/java/com/hazelcast/client/usercodedeployment/ClientUserCodeDeploymentExceptionTest.java index 55b5b1d4f6c3..d4b945f10384 100644 --- a/hazelcast/src/test/java/com/hazelcast/client/usercodedeployment/ClientUserCodeDeploymentExceptionTest.java +++ b/hazelcast/src/test/java/com/hazelcast/client/usercodedeployment/ClientUserCodeDeploymentExceptionTest.java @@ -22,13 +22,13 @@ import com.hazelcast.config.Config; import com.hazelcast.core.HazelcastException; import com.hazelcast.core.HazelcastInstance; -import com.hazelcast.internal.util.FilteringClassLoader; import com.hazelcast.map.IMap; import com.hazelcast.nio.serialization.HazelcastSerializationException; import com.hazelcast.test.HazelcastParallelClassRunner; import com.hazelcast.test.HazelcastTestSupport; import com.hazelcast.test.annotation.ParallelJVMTest; import com.hazelcast.test.annotation.QuickTest; +import com.hazelcast.internal.util.FilteringClassLoader; import org.junit.After; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -38,6 +38,8 @@ import java.io.FileNotFoundException; import static java.util.Collections.singletonList; +import static junit.framework.TestCase.fail; +import static org.junit.Assert.assertEquals; @RunWith(HazelcastParallelClassRunner.class) @Category({QuickTest.class, ParallelJVMTest.class}) @@ -50,7 +52,7 @@ public void tearDown() throws Exception { factory.terminateAll(); } - @Test(expected = HazelcastSerializationException.class) + @Test public void testUserCodeDeploymentIsDisabledByDefaultOnClient() { // this test also validate the EP is filtered locally and has to be loaded from the other member ClientConfig clientConfig = new ClientConfig(); @@ -62,7 +64,12 @@ public void testUserCodeDeploymentIsDisabledByDefaultOnClient() { HazelcastInstance client = factory.newHazelcastClient(clientConfig); IMap map = client.getMap(randomName()); - map.executeOnEntries(incrementingEntryProcessor); + try { + map.executeOnEntries(incrementingEntryProcessor); + fail(); + } catch (HazelcastSerializationException e) { + assertEquals(ClassNotFoundException.class, e.getCause().getClass()); + } } private Config createNodeConfig() { diff --git a/hazelcast/src/test/java/com/hazelcast/internal/util/ExceptionUtilTest.java b/hazelcast/src/test/java/com/hazelcast/internal/util/ExceptionUtilTest.java index 30a980364405..3af582c9b19e 100644 --- a/hazelcast/src/test/java/com/hazelcast/internal/util/ExceptionUtilTest.java +++ b/hazelcast/src/test/java/com/hazelcast/internal/util/ExceptionUtilTest.java @@ -54,16 +54,14 @@ public void testToString() { public void testPeel_whenThrowableIsRuntimeException_thenReturnOriginal() { RuntimeException result = ExceptionUtil.peel(throwable); - assertEquals(throwable.getMessage(), result.getMessage()); - assertEquals(throwable.getClass(), result.getClass()); + assertEquals(throwable, result); } @Test public void testPeel_whenThrowableIsExecutionException_thenReturnCause() { RuntimeException result = ExceptionUtil.peel(new ExecutionException(throwable)); - assertEquals(throwable.getMessage(), result.getMessage()); - assertEquals(throwable.getClass(), result.getClass()); + assertEquals(throwable, result); } @Test diff --git a/hazelcast/src/test/java/com/hazelcast/map/impl/mapstore/writebehind/TransactionsWithWriteBehind_whenNoCoalescingQueueIsFullTest.java b/hazelcast/src/test/java/com/hazelcast/map/impl/mapstore/writebehind/TransactionsWithWriteBehind_whenNoCoalescingQueueIsFullTest.java index cc1e99b137a3..845d11957ffe 100644 --- a/hazelcast/src/test/java/com/hazelcast/map/impl/mapstore/writebehind/TransactionsWithWriteBehind_whenNoCoalescingQueueIsFullTest.java +++ b/hazelcast/src/test/java/com/hazelcast/map/impl/mapstore/writebehind/TransactionsWithWriteBehind_whenNoCoalescingQueueIsFullTest.java @@ -55,6 +55,7 @@ import static com.hazelcast.transaction.TransactionOptions.TransactionType.ONE_PHASE; import static com.hazelcast.transaction.TransactionOptions.TransactionType.TWO_PHASE; import static java.util.concurrent.TimeUnit.SECONDS; +import static org.hamcrest.core.Is.isA; import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; @@ -68,6 +69,7 @@ public class TransactionsWithWriteBehind_whenNoCoalescingQueueIsFullTest extends @Test public void prepare_step_throws_reached_max_size_exception_when_two_phase() { expectedException.expect(TransactionException.class); + expectedException.expectCause(isA(ReachedMaxSizeException.class)); String mapName = "map"; long maxWbqCapacity = 100; diff --git a/hazelcast/src/test/java/com/hazelcast/spi/impl/proxyservice/impl/DistributedObjectFutureTest.java b/hazelcast/src/test/java/com/hazelcast/spi/impl/proxyservice/impl/DistributedObjectFutureTest.java index 8599559ee075..fead90f3e1c7 100644 --- a/hazelcast/src/test/java/com/hazelcast/spi/impl/proxyservice/impl/DistributedObjectFutureTest.java +++ b/hazelcast/src/test/java/com/hazelcast/spi/impl/proxyservice/impl/DistributedObjectFutureTest.java @@ -89,7 +89,7 @@ public void get_throwsGivenException_whenUncheckedExceptionSet() throws Exceptio try { future.get(); } catch (Exception e) { - assertSame(error.getClass(), e.getClass()); + assertSame(error, e); } } From 9e8aeae15772ce773af85bc2d65e315720d69ad9 Mon Sep 17 00:00:00 2001 From: Matko Medenjak Date: Fri, 11 Sep 2020 13:30:53 +0200 Subject: [PATCH 2/2] Revert "Recover exceptions wrapped in same class correctly" This reverts commit 3fb90fdc --- .../client/impl/clientside/ClientExceptionFactory.java | 6 +----- .../client/impl/protocol/ClientExceptionFactoryTest.java | 4 +--- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/hazelcast/src/main/java/com/hazelcast/client/impl/clientside/ClientExceptionFactory.java b/hazelcast/src/main/java/com/hazelcast/client/impl/clientside/ClientExceptionFactory.java index 060ce58f3643..712650fd9f09 100644 --- a/hazelcast/src/main/java/com/hazelcast/client/impl/clientside/ClientExceptionFactory.java +++ b/hazelcast/src/main/java/com/hazelcast/client/impl/clientside/ClientExceptionFactory.java @@ -709,11 +709,7 @@ private Throwable createException(Iterator iterator) { if (exceptionFactory == null) { throwable = new UndefinedErrorCodeException(errorHolder.getMessage(), errorHolder.getClassName()); } else { - Throwable cause = createException(iterator); - throwable = exceptionFactory.createException(errorHolder.getMessage(), cause); - if (throwable.getCause() == null && cause != null) { - throwable.initCause(cause); - } + throwable = exceptionFactory.createException(errorHolder.getMessage(), createException(iterator)); } throwable.setStackTrace(errorHolder.getStackTraceElements().toArray(new StackTraceElement[0])); return throwable; diff --git a/hazelcast/src/test/java/com/hazelcast/client/impl/protocol/ClientExceptionFactoryTest.java b/hazelcast/src/test/java/com/hazelcast/client/impl/protocol/ClientExceptionFactoryTest.java index 413af06df0bc..c96be5a709c3 100644 --- a/hazelcast/src/test/java/com/hazelcast/client/impl/protocol/ClientExceptionFactoryTest.java +++ b/hazelcast/src/test/java/com/hazelcast/client/impl/protocol/ClientExceptionFactoryTest.java @@ -34,7 +34,6 @@ import com.hazelcast.crdt.TargetNotReplicaException; import com.hazelcast.durableexecutor.StaleTaskIdException; import com.hazelcast.internal.cluster.impl.ConfigMismatchException; -import com.hazelcast.internal.util.ExceptionUtil; import com.hazelcast.map.QueryResultSizeExceededException; import com.hazelcast.map.ReachedMaxSizeException; import com.hazelcast.memory.NativeOutOfMemoryError; @@ -273,8 +272,7 @@ public static Iterable parameters() { new Object[]{new IndeterminateOperationStateException(randomString())}, new Object[]{new TargetNotReplicaException(randomString())}, new Object[]{new MutationDisallowedException(randomString())}, - new Object[]{new ConsistencyLostException(randomString())}, - new Object[]{ExceptionUtil.tryWrapInSameClass(new NullPointerException())} + new Object[]{new ConsistencyLostException(randomString())} ); } }