GEODE-9295: Reply sent always while processing LatestLastAccessTimeMe…

…ssage * Even if there any any exception, a reply will be sent back to the sender so that the sender's threads are not stuck.
ringles · Jun 16, 2021 · 31bb9b9 · 31bb9b9
1 parent 790176f
commit 31bb9b9
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 24 deletions.
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessage.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessage.java
@@ -62,34 +62,34 @@ public int getDSFID() {
 
   @Override
   protected void process(ClusterDistributionManager dm) {
-    final InternalCache cache = dm.getCache();
-    if (cache == null) {
-      sendReply(dm, 0);
-      return;
-    }
-    final InternalDistributedRegion region =
-        (InternalDistributedRegion) cache.getRegion(this.regionName);
-    if (region == null) {
-      sendReply(dm, 0);
-      return;
-    }
-    final RegionEntry entry = region.getRegionEntry(this.key);
-    if (entry == null) {
-      sendReply(dm, 0);
-      return;
-    }
     long lastAccessed = 0L;
-    // noinspection SynchronizationOnLocalVariableOrMethodParameter
-    synchronized (entry) {
-      if (!entry.isInvalidOrRemoved()) {
-        try {
-          lastAccessed = entry.getLastAccessed();
-        } catch (InternalStatisticsDisabledException ignored) {
-          // last access time is not available
+    try {
+      final InternalCache cache = dm.getCache();
+      if (cache == null) {
+        return;
+      }
+      final InternalDistributedRegion region =
+          (InternalDistributedRegion) cache.getRegion(this.regionName);
+      if (region == null) {
+        return;
+      }
+      final RegionEntry entry = region.getRegionEntry(this.key);
+      if (entry == null) {
+        return;
+      }
+      // noinspection SynchronizationOnLocalVariableOrMethodParameter
+      synchronized (entry) {
+        if (!entry.isInvalidOrRemoved()) {
+          try {
+            lastAccessed = entry.getLastAccessed();
+          } catch (InternalStatisticsDisabledException ignored) {
+            // last access time is not available
+          }
         }
       }
+    } finally {
+      sendReply(dm, lastAccessed);
     }
-    sendReply(dm, lastAccessed);
   }
 
   @VisibleForTesting

diff --git a/...e-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java b/...e-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java
@@ -14,6 +14,7 @@
  */
 package org.apache.geode.internal.cache;
 
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -77,6 +78,16 @@ public void processWithNullCacheRepliesZero() {
     verify(lastAccessTimeMessage).sendReply(dm, 0);
   }
 
+  @Test
+  public void replyIsSentEvenIfThereIsAnException() {
+    setupMessage();
+    when(dm.getCache()).thenThrow(new RuntimeException());
+    assertThatThrownBy(() -> {
+      lastAccessTimeMessage.process(dm);
+    }).isExactlyInstanceOf(RuntimeException.class);
+    verify(lastAccessTimeMessage).sendReply(dm, 0);
+  }
+
   @Test
   public void processWithNullRegionRepliesZero() {
     setupMessage();