commit 0df32bd60cc6068648c334c36c0ea1c18ed61e8c
author Kirk Lund <klund@apache.org> Thu Oct 24 09:58:57 2019 -0700
committer GitHub <noreply@github.com> Thu Oct 24 09:58:57 2019 -0700
tree 083048d042e800e24dc9fc1f2dfe831db4c62ed6
parent 6ae3f0bc431d681062bec3ad2883d1ef0203a88c

GEODE-7297: Handle more exceptions in FederatingManager (#4208)

Handle or prevent CancelException and NullPointerException in
removeMemberArtifacts.

geode-core/src/main/java/org/apache/geode/management/internal/FederatingManager.java

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
index 7377b62..a885e30 100755
--- 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
@@ -326,12 +326,16 @@
         // ignored
       }
       try {
-        monitoringRegion.localDestroyRegion();
+        if (monitoringRegion != null) {
+          monitoringRegion.localDestroyRegion();
+        }
       } catch (CancelException | RegionDestroyedException ignore) {
         // ignored
       }
       try {
-        notificationRegion.localDestroyRegion();
+        if (notificationRegion != null) {
+          notificationRegion.localDestroyRegion();
+        }
       } catch (CancelException | RegionDestroyedException ignore) {
         // ignored
       }

geode-core/src/test/java/org/apache/geode/management/internal/FederatingManagerTest.java

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
index 2ed7d43..9b02541 100644
--- 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
@@ -15,6 +15,7 @@
 package org.apache.geode.management.internal;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.catchThrowable;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doThrow;
@@ -250,8 +251,6 @@
         .removeAllProxies(member, monitoringRegion);
   }
 
-  // ----------
-
   @Test
   public void removeMemberArtifactsProxyFactoryDoesNotThrowIfCacheClosed() {
     InternalDistributedMember member = member();
@@ -316,8 +315,6 @@
         .localDestroyRegion();
   }
 
-  // ----------
-
   @Test
   public void removeMemberArtifactsProxyFactoryDoesNotThrowIfSystemDisconnected() {
     InternalDistributedMember member = member();
@@ -382,6 +379,42 @@
         .localDestroyRegion();
   }
 
+  @Test
+  public void removeMemberArtifactsDoesNotThrowIfNotificationRegionIsNull() {
+    InternalDistributedMember member = member();
+    when(repo.getEntryFromMonitoringRegionMap(eq(member)))
+        .thenReturn(mock(Region.class));
+    when(repo.getEntryFromNotifRegionMap(eq(member)))
+        .thenReturn(null);
+    when(system.getDistributedMember())
+        .thenReturn(member);
+    FederatingManager federatingManager = new FederatingManager(repo, system, service, cache,
+        statisticsFactory, statisticsClock, proxyFactory, messenger, executorService);
+
+    Throwable thrown = catchThrowable(() -> federatingManager.removeMemberArtifacts(member, false));
+
+    assertThat(thrown)
+        .isNull();
+  }
+
+  @Test
+  public void removeMemberArtifactsDoesNotThrowIfMonitoringRegionIsNull() {
+    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);
+
+    Throwable thrown = catchThrowable(() -> federatingManager.removeMemberArtifacts(member, false));
+
+    assertThat(thrown)
+        .isNull();
+  }
+
   private InternalDistributedMember member() {
     return member(1, 1);
   }