From 5d45e680595ba92ff9f1cef811dcd7c6e3c44086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Mon, 2 Sep 2019 16:04:31 +0200 Subject: [PATCH] cherry-pick #1872 Avoid classloader leak due to static throwables When the stacktrace of a `Throwable` is filled, the JVM transparently adds a `backtrace` field that references classes from the stacktrace. In multi-classloader environments, this can lead to leaks when the `Throwable` (of subclass of) is static. This commit changes the few occurrences of static exceptions in Reactor so that they each use a variant that doesn't fill the stack trace. TERMINATED uses a general-purpose StaticThrowable which means that it can reference the super constructor with boolean flags disabling exception suppression and stack trace filling, without having to override the fillInStackTrace method. Reviewed-in: #1876 --- .../main/java/reactor/core/Exceptions.java | 54 +++++++++++++++++-- .../java/reactor/core/ExceptionsTest.java | 6 +-- 2 files changed, 52 insertions(+), 8 deletions(-) diff --git a/reactor-core/src/main/java/reactor/core/Exceptions.java b/reactor-core/src/main/java/reactor/core/Exceptions.java index 93fef6f1ce..edbdf0f979 100644 --- a/reactor-core/src/main/java/reactor/core/Exceptions.java +++ b/reactor-core/src/main/java/reactor/core/Exceptions.java @@ -44,7 +44,7 @@ public abstract class Exceptions { * don't leak this! */ @SuppressWarnings("ThrowableInstanceNeverThrown") - public static final Throwable TERMINATED = new Throwable("Operator has been terminated"); + public static final Throwable TERMINATED = new StaticThrowable("Operator has been terminated"); /** * Update an empty atomic reference with the given exception, or combine further added @@ -499,11 +499,10 @@ public static final Throwable addSuppressed(Throwable original, final Throwable Exceptions() { } - static final RejectedExecutionException REJECTED_EXECUTION = new RejectedExecutionException("Scheduler unavailable"); + static final RejectedExecutionException REJECTED_EXECUTION = new StaticRejectedExecutionException("Scheduler unavailable"); static final RejectedExecutionException NOT_TIME_CAPABLE_REJECTED_EXECUTION = - new RejectedExecutionException( - "Scheduler is not capable of time-based scheduling"); + new StaticRejectedExecutionException("Scheduler is not capable of time-based scheduling"); static class CompositeException extends ReactiveException { @@ -586,11 +585,56 @@ static final class OverflowException extends IllegalStateException { } } - static final class ReactorRejectedExecutionException extends RejectedExecutionException { + static class ReactorRejectedExecutionException extends RejectedExecutionException { ReactorRejectedExecutionException(String message, Throwable cause) { super(message, cause); } + + ReactorRejectedExecutionException(String message) { + super(message); + } + } + + /** + * A {@link RejectedExecutionException} that is tailored for usage as a static final + * field. It avoids {@link ClassLoader}-related leaks by bypassing stacktrace filling. + */ + static final class StaticRejectedExecutionException extends RejectedExecutionException { + + StaticRejectedExecutionException(String message, Throwable cause) { + super(message, cause); + } + + StaticRejectedExecutionException(String message) { + super(message); + } + + @Override + public synchronized Throwable fillInStackTrace() { + return this; + } + } + + /** + * A general-purpose {@link Throwable} that is suitable for usage as a static final + * field. It avoids {@link ClassLoader}-related leaks by bypassing stacktrace filling. + * Exception {{@link Exception#addSuppressed(Throwable)} suppression} is also disabled. + */ + //see https://github.com/reactor/reactor-core/pull/1872 + static final class StaticThrowable extends Error { + + StaticThrowable(String message) { + super(message, null, false, false); + } + + StaticThrowable(String message, Throwable cause) { + super(message, cause, false, false); + } + + StaticThrowable(Throwable cause) { + super(cause.toString(), cause, false, false); + } } } diff --git a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java index 00fe6fb329..2e447c4303 100644 --- a/reactor-core/src/test/java/reactor/core/ExceptionsTest.java +++ b/reactor-core/src/test/java/reactor/core/ExceptionsTest.java @@ -296,7 +296,7 @@ public void failWithRejectedNormalWraps() { } @Test - public void failWithRejectedSingletonReeWraps() { + public void failWithRejectedSingletonREEWraps() { Throwable test = REJECTED_EXECUTION; assertThat(Exceptions.failWithRejected(test)) .isInstanceOf(Exceptions.ReactorRejectedExecutionException.class) @@ -305,7 +305,7 @@ public void failWithRejectedSingletonReeWraps() { } @Test - public void failWithRejectedNormalReeWraps() { + public void failWithRejectedNormalREEWraps() { Throwable test = new RejectedExecutionException("boom"); assertThat(Exceptions.failWithRejected(test)) .isInstanceOf(Exceptions.ReactorRejectedExecutionException.class) @@ -314,7 +314,7 @@ public void failWithRejectedNormalReeWraps() { } @Test - public void failWithRejectedReactorReeNoOp() { + public void failWithRejectedReactorREENoOp() { Throwable test = new Exceptions.ReactorRejectedExecutionException("boom", REJECTED_EXECUTION); assertThat(Exceptions.failWithRejected(test)) .isSameAs(test)