[geode] 01/02: GEODE-9295: Reply sent always while processing LatestLastAccessTimeMessage

[nnag]

This is an automated email from the ASF dual-hosted git repository.

nnag pushed a commit to branch support/1.13
in repository https://gitbox.apache.org/repos/asf/geode.git

commit 3a88dc11b08fc9ec18f2724568e950d61e77c84a
Author: Nabarun Nag <n...@cs.wisc.edu>
AuthorDate: Mon Jun 14 22:17:36 2021 -0700

    GEODE-9295: Reply sent always  while processing LatestLastAccessTimeMessage
    
        * Even if there any any exception, a reply will be sent back to the 
sender so that the sender's threads are not stuck.
    
    (cherry picked from commit 31bb9b986ed5b1a8013af35b277147e28cd74d12)
---
 .../cache/LatestLastAccessTimeMessage.java         | 48 +++++++++++-----------
 .../cache/LatestLastAccessTimeMessageTest.java     | 11 +++++
 2 files changed, 35 insertions(+), 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
index 46bb749..d90be52 100644
--- 
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 class LatestLastAccessTimeMessage<K> extends 
PooledDistributionMessage
 
   @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/geode-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java
 
b/geode-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java
index 380006e..9f5062c 100644
--- 
a/geode-core/src/test/java/org/apache/geode/internal/cache/LatestLastAccessTimeMessageTest.java
+++ 
b/geode-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;
@@ -78,6 +79,16 @@ public class LatestLastAccessTimeMessageTest {
   }
 
   @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();
     setupRegion(false, false);