From 75a0ab0574aa5addc3f5da85247019dd81bc7393 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Fri, 15 Dec 2017 12:30:59 +0100 Subject: [PATCH] fix #985 Prevent NPE when debug mode is activated via command line This commit fixes an NPE and subsequent initialization error when the debug mode is activated through the `reactor.trace.operatorStacktrace` System property, by correctly reordering the static fields and blocks. Additionally, the debug mode via command line also uses the correct hook key, so that activating via System property then deactivating via Hooks method would work. Finally, this behavior is tested as an additional test task in Gradle, ensuring the property is set and no previous classloading of Hooks has been done by other tests. --- build.gradle | 9 +++ .../java/reactor/core/publisher/Hooks.java | 57 +++++++++------- .../core/publisher/HooksTestStaticInit.java | 67 +++++++++++++++++++ 3 files changed, 107 insertions(+), 26 deletions(-) create mode 100644 reactor-core/src/test/java/reactor/core/publisher/HooksTestStaticInit.java diff --git a/build.gradle b/build.gradle index 89b2fc0204..2a1361a70d 100644 --- a/build.gradle +++ b/build.gradle @@ -354,6 +354,15 @@ project('reactor-core') { } } + task testStaticInit(type: Test, group: 'verification') { + systemProperty 'reactor.trace.operatorStacktrace', 'true' + includes.clear() + include '**/*TestStaticInit.*' + doFirst { + println "Additional tests from `testStaticInit` ($includes)" + } + } + sourceSets.test.resources.srcDirs = ["src/test/resources", "src/test/java"] if (!JavaVersion.current().isJava9Compatible()) { diff --git a/reactor-core/src/main/java/reactor/core/publisher/Hooks.java b/reactor-core/src/main/java/reactor/core/publisher/Hooks.java index e7122f02f8..fca1a24f85 100644 --- a/reactor-core/src/main/java/reactor/core/publisher/Hooks.java +++ b/reactor-core/src/main/java/reactor/core/publisher/Hooks.java @@ -463,6 +463,36 @@ static Function createOrUpdateOpHook(Collection(1); onLastOperatorHooks = new LinkedHashMap<>(1); @@ -473,7 +503,7 @@ static Function createOrUpdateOpHook(Collection apply(Publisher publisher) { } } - static final Logger log = Loggers.getLogger(Hooks.class); - - /** - * A key that can be used to store a sequence-specific {@link Hooks#onErrorDropped(Consumer)} - * hook in a {@link Context}, as a {@link Consumer Consumer<Throwable>}. - */ - static final String KEY_ON_ERROR_DROPPED = "reactor.onErrorDropped.local"; - /** - * A key that can be used to store a sequence-specific {@link Hooks#onNextDropped(Consumer)} - * hook in a {@link Context}, as a {@link Consumer Consumer<Object>}. - */ - static final String KEY_ON_NEXT_DROPPED = "reactor.onNextDropped.local"; - /** - * A key that can be used to store a sequence-specific {@link Hooks#onOperatorError(BiFunction)} - * hook in a {@link Context}, as a {@link BiFunction BiFunction<Throwable, Object, Throwable>}. - */ - static final String KEY_ON_OPERATOR_ERROR = "reactor.onOperatorError.local"; - /** - * A key that can be used to store a sequence-specific {@link Hooks#onOperatorError(BiFunction)} - * hook THAT IS ONLY APPLIED TO Operators{@link Operators#onRejectedExecution(Throwable, Context) onRejectedExecution} - * in a {@link Context}, as a {@link BiFunction BiFunction<Throwable, Object, Throwable>}. - */ - static final String KEY_ON_REJECTED_EXECUTION = "reactor.onRejectedExecution.local"; - - static final String ON_OPERATOR_DEBUG_KEY = "onOperatorDebug"; } diff --git a/reactor-core/src/test/java/reactor/core/publisher/HooksTestStaticInit.java b/reactor-core/src/test/java/reactor/core/publisher/HooksTestStaticInit.java new file mode 100644 index 0000000000..dc273e2078 --- /dev/null +++ b/reactor-core/src/test/java/reactor/core/publisher/HooksTestStaticInit.java @@ -0,0 +1,67 @@ +/* + * Copyright (c) 2011-2017 Pivotal Software Inc, All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/* + * Copyright (c) 2011-2017 Pivotal Software Inc, All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package reactor.core.publisher; + +import org.junit.After; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class HooksTestStaticInit { + + //IMPORTANT: this test case depends on System property + // `reactor.trace.operatorStacktrace` be set to `true` + + @After + public void resetAllHooks() { + Hooks.resetOnOperatorError(); + Hooks.resetOnNextDropped(); + Hooks.resetOnErrorDropped(); +// Hooks.resetOnOperatorDebug(); //superseded by resetOnEachOperator + Hooks.resetOnEachOperator(); + Hooks.resetOnLastOperator(); + } + + @Test + public void syspropDebugModeShouldNotFail() { + assertThat(System.getProperties()) + .as("debug mode set via system property") + .containsEntry("reactor.trace.operatorStacktrace", "true"); + assertThat(Hooks.getOnEachOperatorHooks()) + .as("debug hook activated") + .containsKey(Hooks.ON_OPERATOR_DEBUG_KEY); + //would throw NPE due to https://github.com/reactor/reactor-core/issues/985 + Mono.just("hello").subscribe(); + } + +}