This is an automated email from the ASF dual-hosted git repository.
eshu11 pushed a commit to branch feature/GEODE-3190
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/feature/GEODE-3190 by this
push:
new 630e080 GEODE-3190: Fix a race by checking if region is destroyed
630e080 is described below
commit 630e0801b048a4611e30d58975e8b30475dbe946
Author: eshu <[email protected]>
AuthorDate: Thu Oct 26 16:36:13 2017 -0700
GEODE-3190: Fix a race by checking if region is destroyed
* Add test case for heap region as well as offheap region.
* Rename the test.
---
.../geode/internal/cache/AbstractRegionMap.java | 8 ++-
...troyEntryWithConcurrentOperationJUnitTest.java} | 74 +++++++++++++---------
2 files changed, 49 insertions(+), 33 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
index ea26ef9..0700c31 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/AbstractRegionMap.java
@@ -97,7 +97,7 @@ public abstract class AbstractRegionMap implements RegionMap {
* This test hook is used to force the conditions for defect 48182. This
hook is used by
* Bug48182JUnitTest.
*/
- static Runnable testHookRunnableFor48182 = null;
+ static Runnable testHookRunnableForConcurrentOperation = null;
private RegionEntryFactory entryFactory;
@@ -1070,8 +1070,8 @@ public abstract class AbstractRegionMap implements
RegionMap {
/*
* Execute the test hook runnable inline (not threaded) if it is not
null.
*/
- if (null != testHookRunnableFor48182) {
- testHookRunnableFor48182.run();
+ if (null != testHookRunnableForConcurrentOperation) {
+ testHookRunnableForConcurrentOperation.run();
}
try {
@@ -1388,6 +1388,7 @@ public abstract class AbstractRegionMap implements
RegionMap {
}
try {
synchronized (re) {
+ owner.checkReadiness();
// if the entry is a tombstone and the event is from a peer or
a client
// then we allow the operation to be performed so that we can
update the
// version stamp. Otherwise we would retain an old version
stamp and may allow
@@ -1489,6 +1490,7 @@ public abstract class AbstractRegionMap implements
RegionMap {
duringRI, true);
doPart3 = true;
} finally {
+ owner.checkReadiness();
if (re.isRemoved() && !re.isTombstone()) {
if (!removed) {
removeEntry(event.getKey(), re, true, event, owner);
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/Bug48182JUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/DestroyEntryWithConcurrentOperationJUnitTest.java
similarity index 71%
rename from
geode-core/src/test/java/org/apache/geode/internal/cache/Bug48182JUnitTest.java
rename to
geode-core/src/test/java/org/apache/geode/internal/cache/DestroyEntryWithConcurrentOperationJUnitTest.java
index c6a3354..8650e90 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/Bug48182JUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/DestroyEntryWithConcurrentOperationJUnitTest.java
@@ -30,12 +30,11 @@ import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
/**
- * TestCase that emulates the conditions that produce defect 48182 and ensures
that the fix works
- * under those conditions. 48182: Unexpected EntryNotFoundException while
shutting down members with
- * off-heap https://svn.gemstone.com/trac/gemfire/ticket/48182
+ * TestCase that emulates the conditions that entry destroy with concurrent
destroy region or cache
+ * close event will get expected Exception.
*/
@Category(IntegrationTest.class)
-public class Bug48182JUnitTest {
+public class DestroyEntryWithConcurrentOperationJUnitTest {
/**
* A region entry key.
*/
@@ -105,8 +104,8 @@ public class Bug48182JUnitTest {
/**
* Creates and returns the test region with concurrency checks enabled.
*/
- protected Region<Object, Object> createRegion() {
- return createRegion(true);
+ protected Region<Object, Object> createRegion(boolean isOffHeap) {
+ return createRegion(true, isOffHeap);
}
/**
@@ -114,8 +113,9 @@ public class Bug48182JUnitTest {
*
* @param concurrencyChecksEnabled concurrency checks will be enabled if
true.
*/
- protected Region<Object, Object> createRegion(boolean
concurrencyChecksEnabled) {
- return getCache().createRegionFactory(getRegionShortcut()).setOffHeap(true)
+ protected Region<Object, Object> createRegion(boolean
concurrencyChecksEnabled,
+ boolean isOffHeap) {
+ return
getCache().createRegionFactory(getRegionShortcut()).setOffHeap(isOffHeap)
.setConcurrencyChecksEnabled(concurrencyChecksEnabled).create(getRegionName());
}
@@ -131,15 +131,24 @@ public class Bug48182JUnitTest {
return result;
}
+ @Test
+ public void testEntryDestroyWithCacheClose() throws Exception {
+ testEntryDestroyWithCacheClose(false);
+ }
+
+ @Test
+ public void testOffHeapRegionEntryDestroyWithCacheClose() throws Exception {
+ testEntryDestroyWithCacheClose(true);
+ }
+
/**
- * Simulates the conditions for 48182 by setting a test hook boolean in
{@link AbstractRegionMap}.
- * This test hook forces a cache close during a destroy in an off-heap
region. This test asserts
- * that a CacheClosedException is thrown rather than an
EntryNotFoundException (or any other
- * exception type for that matter).
+ * Simulates the conditions setting a test hook boolean in {@link
AbstractRegionMap}. This test
+ * hook forces a cache close during a destroy in a region. This test asserts
that a
+ * CacheClosedException is thrown rather than an EntryNotFoundException (or
any other exception
+ * type for that matter).
*/
- @Test
- public void test48182WithCacheClose() throws Exception {
- AbstractRegionMap.testHookRunnableFor48182 = new Runnable() {
+ private void testEntryDestroyWithCacheClose(boolean isOffHeap) {
+ AbstractRegionMap.testHookRunnableForConcurrentOperation = new Runnable() {
@Override
public void run() {
getCache().close();
@@ -149,7 +158,7 @@ public class Bug48182JUnitTest {
// True when the correct exception has been triggered.
boolean correctException = false;
- Region<Object, Object> region = createRegion();
+ Region<Object, Object> region = createRegion(isOffHeap);
region.put(KEY, VALUE);
try {
@@ -162,30 +171,35 @@ public class Bug48182JUnitTest {
fail("Did not receive a CacheClosedException. Received a " +
e.getClass().getName()
+ " instead.");
} finally {
- AbstractRegionMap.testHookRunnableFor48182 = null;
+ AbstractRegionMap.testHookRunnableForConcurrentOperation = null;
}
assertTrue("A CacheClosedException was not triggered", correctException);
}
+ @Test
+ public void testEntryDestroyWithRegionDestroy() throws Exception {
+ verifyConcurrentRegionDestroyWithEntryDestroy(false);
+ }
+
+ @Test
+ public void testEntryDestroyWithOffHeapRegionDestroy() throws Exception {
+ verifyConcurrentRegionDestroyWithEntryDestroy(true);
+ }
+
/**
- * Simulates the conditions similar to 48182 by setting a test hook boolean
in
- * {@link AbstractRegionMap}. This test hook forces a region destroy during
a destroy operation in
- * an off-heap region. This test asserts that a RegionDestroyedException is
thrown rather than an
- * EntryNotFoundException (or any other exception type for that matter).
+ * Simulates the conditions by setting a test hook boolean in {@link
AbstractRegionMap}. This test
+ * hook forces a region destroy during a destroy operation in a region. This
test asserts that a
+ * RegionDestroyedException is thrown rather than an EntryNotFoundException
(or any other
+ * exception type for that matter).
*/
- @Test
- public void test48182WithRegionDestroy() throws Exception {
- AbstractRegionMap.testHookRunnableFor48182 = new Runnable() {
+ private void verifyConcurrentRegionDestroyWithEntryDestroy(boolean
isOffHeap) {
+ AbstractRegionMap.testHookRunnableForConcurrentOperation = new Runnable() {
@Override
public void run() {
Cache cache = getCache();
Region region = cache.getRegion(getRegionName());
region.destroyRegion();
- if (cache.getLogger() != null) {
- cache.getLogger()
- .info("Region " + getRegionName() + " is destroyed : " +
region.isDestroyed());
- }
assertTrue("Region " + getRegionName() + " is not destroyed.",
region.isDestroyed());
}
};
@@ -193,7 +207,7 @@ public class Bug48182JUnitTest {
// True when the correct exception has been triggered.
boolean correctException = false;
- Region<Object, Object> region = createRegion();
+ Region<Object, Object> region = createRegion(isOffHeap);
region.put(KEY, VALUE);
try {
@@ -206,7 +220,7 @@ public class Bug48182JUnitTest {
fail("Did not receive a RegionDestroyedException. Received a " +
e.getClass().getName()
+ " instead.");
} finally {
- AbstractRegionMap.testHookRunnableFor48182 = null;
+ AbstractRegionMap.testHookRunnableForConcurrentOperation = null;
}
assertTrue("A RegionDestroyedException was not triggered",
correctException);
--
To stop receiving notification emails like this one, please contact
['"[email protected]" <[email protected]>'].
