From 31bb9b986ed5b1a8013af35b277147e28cd74d12 Mon Sep 17 00:00:00 2001
From: Nabarun Nag <nag@cs.wisc.edu>
Date: Mon, 14 Jun 2021 22:17:36 -0700
Subject: [PATCH] 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.
---
 .../cache/LatestLastAccessTimeMessage.java    | 48 +++++++++----------
 .../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 46bb74963eae..d90be522ff24 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 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/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 380006e0ad9f..9f5062c150bf 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;
@@ -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();