[geode] branch develop updated: GEODE-5592: Release the lock in a finally block. (#2344)

[es...@apache.org]

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

eshu11 pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 6fa2d3e  GEODE-5592: Release the lock in a finally block. (#2344)
6fa2d3e is described below

commit 6fa2d3efb30bf43eda0f21326dfdcbb5d6f2dfc4
Author: pivotal-eshu <es...@pivotal.io>
AuthorDate: Fri Aug 17 10:31:59 2018 -0700

    GEODE-5592: Release the lock in a finally block. (#2344)
---
 .../apache/geode/internal/cache/TXManagerImpl.java | 11 ++++++----
 .../geode/internal/cache/TXManagerImplTest.java    | 25 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
index 9f6b87b..b0ec44d 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TXManagerImpl.java
@@ -1018,13 +1018,16 @@ public class TXManagerImpl implements CacheTransactionManager, MembershipListene
       if (tx.isOnBehalfOfClient()) {
         updateLastOperationTime(tx);
       }
-      cleanupTransactionIfNoLongerHost(tx);
-      setTXState(null);
-      tx.getLock().unlock();
+      try {
+        cleanupTransactionIfNoLongerHost(tx);
+      } finally {
+        setTXState(null);
+        tx.getLock().unlock();
+      }
     }
   }
 
-  private void cleanupTransactionIfNoLongerHost(TXStateProxy tx) {
+  void cleanupTransactionIfNoLongerHost(TXStateProxy tx) {
     synchronized (hostedTXStates) {
       if (!hostedTXStates.containsKey(tx.getTxId())) {
         // clean up the transaction if no longer the host of the transaction
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/TXManagerImplTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/TXManagerImplTest.java
index 77c67e5..3669400 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/TXManagerImplTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/TXManagerImplTest.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
@@ -37,6 +38,7 @@ import java.util.Iterator;
 import java.util.Set;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.awaitility.Awaitility;
 import org.junit.Before;
@@ -495,4 +497,27 @@ public class TXManagerImplTest {
 
     verify(timer, times(1)).schedule(any(), eq(1000L));
   }
+
+  @Test
+  public void unmasqueradeReleasesTheLockHeld() {
+    tx1 = mock(TXStateProxyImpl.class);
+    ReentrantLock lock = mock(ReentrantLock.class);
+    when(tx1.getLock()).thenReturn(lock);
+
+    spyTxMgr.unmasquerade(tx1);
+
+    verify(lock, times(1)).unlock();
+  }
+
+  @Test(expected = RuntimeException.class)
+  public void unmasqueradeReleasesTheLockHeldWhenCleanupTransactionIfNoLongerHostFailedWithException() {
+    tx1 = mock(TXStateProxyImpl.class);
+    ReentrantLock lock = mock(ReentrantLock.class);
+    when(tx1.getLock()).thenReturn(lock);
+    doThrow(new RuntimeException()).when(spyTxMgr).cleanupTransactionIfNoLongerHost(tx1);
+
+    spyTxMgr.unmasquerade(tx1);
+
+    verify(lock, times(1)).unlock();
+  }
 }