This is an automated email from the ASF dual-hosted git repository. klund pushed a commit to branch GEODE-6959-AlertAppender-missing-NPE in repository https://gitbox.apache.org/repos/asf/geode.git
commit 5be6628bdce0b04b2cf299efe1becb163ba6bc32 Author: Kirk Lund <[email protected]> AuthorDate: Thu Aug 8 10:36:28 2019 -0700 GEODE-6959: Prevent NPE in GMSMembershipManager when AlertAppender is not set Co-authored-by: Mark Hanson <[email protected]> Co-authored-by: Kirk Lund <[email protected]> --- .../membership/adapter/GMSMembershipManager.java | 2 +- .../internal/logging/log4j/AlertAppender.java | 16 ++++++++++++- .../internal/logging/log4j/AlertAppenderTest.java | 26 ++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/adapter/GMSMembershipManager.java b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/adapter/GMSMembershipManager.java index 881866c..8db6962 100644 --- a/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/adapter/GMSMembershipManager.java +++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/membership/adapter/GMSMembershipManager.java @@ -2584,7 +2584,7 @@ public class GMSMembershipManager implements MembershipManager { services.setShutdownCause(shutdownCause); services.getCancelCriterion().cancel(reason); - AlertAppender.getInstance().stopSession(); + AlertAppender.stopSessionIfRunning(); if (!inhibitForceDisconnectLogging) { logger.fatal( diff --git a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java index d56a0da..40332da 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java +++ b/geode-core/src/main/java/org/apache/geode/internal/logging/log4j/AlertAppender.java @@ -36,6 +36,7 @@ import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; +import org.apache.geode.annotations.VisibleForTesting; import org.apache.geode.annotations.internal.MakeNotStatic; import org.apache.geode.distributed.DistributedMember; import org.apache.geode.internal.alerting.AlertLevel; @@ -337,7 +338,20 @@ public class AlertAppender extends AbstractAppender return listeners; } - public static AlertAppender getInstance() { + @VisibleForTesting + static AlertAppender getInstance() { return instanceRef.get(); } + + @VisibleForTesting + static void setInstance(AlertAppender alertAppender) { + instanceRef.set(alertAppender); + } + + public static void stopSessionIfRunning() { + AlertAppender instance = instanceRef.get(); + if (instance != null) { + instance.stopSession(); + } + } } diff --git a/geode-core/src/test/java/org/apache/geode/internal/logging/log4j/AlertAppenderTest.java b/geode-core/src/test/java/org/apache/geode/internal/logging/log4j/AlertAppenderTest.java index 0ba7e45..6779b20 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/logging/log4j/AlertAppenderTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/logging/log4j/AlertAppenderTest.java @@ -15,9 +15,13 @@ package org.apache.geode.internal.logging.log4j; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import org.apache.logging.log4j.Level; +import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -52,6 +56,11 @@ public class AlertAppenderTest { asAlertingProvider = alertAppender; } + @After + public void tearDown() { + AlertAppender.setInstance(null); + } + @Test public void alertListenersIsEmptyByDefault() { assertThat(alertAppender.getAlertListeners()).isEmpty(); @@ -164,4 +173,21 @@ public class AlertAppenderTest { assertThat(alertAppender.getAlertListeners()).containsExactly(listener1, listener2, listener3); } + + @Test + public void stopSessionIfRunningDoesNotThrowIfReferenceIsNull() { + AlertAppender.setInstance(null); + + assertThatCode(AlertAppender::stopSessionIfRunning).doesNotThrowAnyException(); + } + + @Test + public void stopSessionIfRunningStopCurrentInstance() { + alertAppender = spy(alertAppender); + AlertAppender.setInstance(alertAppender); + + AlertAppender.stopSessionIfRunning(); + + verify(alertAppender).stopSession(); + } }
