You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by es...@apache.org on 2018/08/16 23:36:42 UTC
[geode] branch feature/GEODE-5592 updated: GEODE-5592: Release the
lock in a finally block.
This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-5592
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-5592 by this push:
new c992f4d GEODE-5592: Release the lock in a finally block.
c992f4d is described below
commit c992f4ddf0563e513f1ad45359eba3c06b8683a0
Author: eshu <es...@pivotal.io>
AuthorDate: Thu Aug 16 16:34:02 2018 -0700
GEODE-5592: Release the lock in a finally block.
---
.../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();
+ }
}