GEODE-7525: Prevent NPE in MBeanProxyFactory (apache#4478)

FederatingManager should not invoke MBeanProxyFactory.removeAllProxies when monitoringRegion is null.
ybz216 · Dec 17, 2019 · 9934558 · 9934558
1 parent 7187192
commit 9934558
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 5 deletions.
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java b/geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java
@@ -334,18 +334,15 @@ void removeMemberArtifacts(DistributedMember member, boolean crashed) {
 
     // If cache is closed all the regions would have been destroyed implicitly
     if (!cache.isClosed()) {
-      try {
-        proxyFactory.removeAllProxies(member, monitoringRegion);
-      } catch (CancelException | RegionDestroyedException ignore) {
-        // ignored
-      }
       try {
         if (monitoringRegion != null) {
+          proxyFactory.removeAllProxies(member, monitoringRegion);
           monitoringRegion.localDestroyRegion();
         }
       } catch (CancelException | RegionDestroyedException ignore) {
         // ignored
       }
+
       try {
         if (notificationRegion != null) {
           notificationRegion.localDestroyRegion();

diff --git a/geode-core/src/test/java/org/apache/geode/management/internal/FederatingManagerTest.java b/geode-core/src/test/java/org/apache/geode/management/internal/FederatingManagerTest.java
@@ -21,6 +21,7 @@
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.powermock.api.mockito.PowerMockito.when;
 
 import java.net.InetAddress;
@@ -429,6 +430,23 @@ public void startManagerGetsNewExecutorServiceFromSupplier() {
     verify(executorServiceSupplier).get();
   }
 
+  @Test
+  public void removeMemberArtifactsDoesNotRemoveAllProxiesIfMonitoringRegionIsNull() {
+    InternalDistributedMember member = member();
+    when(repo.getEntryFromMonitoringRegionMap(eq(member)))
+        .thenReturn(null);
+    when(repo.getEntryFromNotifRegionMap(eq(member)))
+        .thenReturn(mock(Region.class));
+    when(system.getDistributedMember())
+        .thenReturn(member);
+    FederatingManager federatingManager = new FederatingManager(repo, system, service, cache,
+        statisticsFactory, statisticsClock, proxyFactory, messenger, executorService);
+
+    federatingManager.removeMemberArtifacts(member, false);
+
+    verifyZeroInteractions(proxyFactory);
+  }
+
   private InternalDistributedMember member() {
     return member(1, 1);
   }