This is an automated email from the ASF dual-hosted git repository. eshu11 pushed a commit to branch feature/GEODE-6638 in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-6638 by this push: new 5a6b9cb GEODE-6638: do not fail with IllegalMonitorStateException during cache close 5a6b9cb is described below commit 5a6b9cb64e903dec1310f9d7fc5ca99b4dcdb082 Author: eshu <e...@pivotal.io> AuthorDate: Thu Apr 11 16:30:51 2019 -0700 GEODE-6638: do not fail with IllegalMonitorStateException during cache close --- .../org/apache/geode/internal/cache/TXState.java | 12 ++-- .../apache/geode/internal/cache/TXStateTest.java | 74 ++++++++++++++++++++++ 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java b/geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java index f8671a2..1611da4 100644 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TXState.java @@ -876,7 +876,7 @@ public class TXState implements TXStateInterface { } void doCleanup() { - IllegalArgumentException iae = null; + RuntimeException exception = null; try { this.closed = true; this.seenEvents.clear(); @@ -886,8 +886,8 @@ public class TXState implements TXStateInterface { final long conflictStart = CachePerfStats.getStatTime(); try { this.locks.cleanup(getCache().getInternalDistributedSystem()); - } catch (IllegalArgumentException e) { - iae = e; + } catch (IllegalArgumentException | IllegalMonitorStateException e) { + exception = e; } if (CachePerfStats.enableClockStats) this.proxy.getTxMgr().getCachePerfStats() @@ -918,8 +918,6 @@ public class TXState implements TXStateInterface { "Exception while unlocking bucket region {} this is probably because the bucket was destroyed and never locked initially.", r.getFullPath(), rde); } - } finally { - } } } @@ -930,8 +928,8 @@ public class TXState implements TXStateInterface { this.completionGuard.notifyAll(); } - if (iae != null && !this.proxy.getCache().isClosed()) { - throw iae; + if (exception != null && !this.proxy.getCache().isClosed()) { + throw exception; } } } diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/TXStateTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/TXStateTest.java index fad1973..e49f423 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/TXStateTest.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/TXStateTest.java @@ -40,12 +40,15 @@ import org.apache.geode.cache.RegionAttributes; import org.apache.geode.cache.SynchronizationCommitConflictException; import org.apache.geode.cache.TransactionDataNodeHasDepartedException; import org.apache.geode.cache.TransactionException; +import org.apache.geode.distributed.internal.InternalDistributedSystem; public class TXStateTest { private TXStateProxyImpl txStateProxy; private CommitConflictException exception; private TransactionDataNodeHasDepartedException transactionDataNodeHasDepartedException; private SingleThreadJTAExecutor executor; + private InternalCache cache; + private InternalDistributedSystem internalDistributedSystem; @Before public void setup() { @@ -53,8 +56,11 @@ public class TXStateTest { exception = new CommitConflictException(""); transactionDataNodeHasDepartedException = new TransactionDataNodeHasDepartedException(""); executor = mock(SingleThreadJTAExecutor.class); + cache = mock(InternalCache.class); + internalDistributedSystem = mock(InternalDistributedSystem.class); when(txStateProxy.getTxMgr()).thenReturn(mock(TXManagerImpl.class)); + when(cache.getInternalDistributedSystem()).thenReturn(internalDistributedSystem); } @Test @@ -223,4 +229,72 @@ public class TXStateTest { verify(txRegionState, never()).cleanupNonDirtyEntries(internalRegion); } + @Test + public void doCleanupContinuesWhenReleasingLockGotIllegalArgumentExceptionIfCacheIsClosing() { + TXState txState = spy(new TXState(txStateProxy, false)); + txState.locks = mock(TXLockRequest.class); + doReturn(cache).when(txStateProxy).getCache(); + doThrow(new IllegalArgumentException()).when(txState.locks).cleanup(internalDistributedSystem); + when(cache.isClosed()).thenReturn(true); + TXRegionState regionState1 = mock(TXRegionState.class); + InternalRegion region1 = mock(InternalRegion.class); + txState.regions.put(region1, regionState1); + + txState.doCleanup(); + + verify(regionState1).cleanup(region1); + } + + @Test + public void doCleanupContinuesWhenReleasingLockGotIllegalMonitorStateExceptionIfCacheIsClosing() { + TXState txState = spy(new TXState(txStateProxy, false)); + txState.locks = mock(TXLockRequest.class); + doReturn(cache).when(txStateProxy).getCache(); + doThrow(new IllegalMonitorStateException()).when(txState.locks) + .cleanup(internalDistributedSystem); + when(cache.isClosed()).thenReturn(true); + TXRegionState regionState1 = mock(TXRegionState.class); + InternalRegion region1 = mock(InternalRegion.class); + txState.regions.put(region1, regionState1); + + txState.doCleanup(); + + verify(regionState1).cleanup(region1); + } + + @Test + public void doCleanupThrowsWhenReleasingLockGotIllegalArgumentExceptionIfCacheIsNotClosing() { + TXState txState = spy(new TXState(txStateProxy, false)); + txState.locks = mock(TXLockRequest.class); + doReturn(cache).when(txStateProxy).getCache(); + doThrow(new IllegalArgumentException()).when(txState.locks).cleanup(internalDistributedSystem); + when(cache.isClosed()).thenReturn(false); + TXRegionState regionState1 = mock(TXRegionState.class); + InternalRegion region1 = mock(InternalRegion.class); + txState.regions.put(region1, regionState1); + + Throwable thrown = catchThrowable(() -> txState.doCleanup()); + + assertThat(thrown).isInstanceOf(IllegalArgumentException.class); + verify(regionState1).cleanup(region1); + } + + @Test + public void doCleanupThrowsWhenReleasingLockGotIllegalMonitorStateExceptionIfCacheIsNotClosing() { + TXState txState = spy(new TXState(txStateProxy, false)); + txState.locks = mock(TXLockRequest.class); + doReturn(cache).when(txStateProxy).getCache(); + doThrow(new IllegalMonitorStateException()).when(txState.locks) + .cleanup(internalDistributedSystem); + when(cache.isClosed()).thenReturn(false); + TXRegionState regionState1 = mock(TXRegionState.class); + InternalRegion region1 = mock(InternalRegion.class); + txState.regions.put(region1, regionState1); + + Throwable thrown = catchThrowable(() -> txState.doCleanup()); + + assertThat(thrown).isInstanceOf(IllegalMonitorStateException.class); + verify(regionState1).cleanup(region1); + } + }