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 e630cd3 GEODE-3190: Fix a race by checking if region is destroyed
(#984)
e630cd3 is described below
commit e630cd3ad93f84a14819308d7a97d9bb9fb7197f
Author: pivotal-eshu <[email protected]>
AuthorDate: Fri Oct 27 17:09:57 2017 -0700
GEODE-3190: Fix a race by checking if region is destroyed (#984)
* 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 | 12 ++--
...troyEntryWithConcurrentOperationJUnitTest.java} | 74 +++++++++++++---------
2 files changed, 51 insertions(+), 35 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..52d6e62 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
@@ -94,10 +94,10 @@ public abstract class AbstractRegionMap implements
RegionMap {
protected CustomEntryConcurrentHashMap<Object, Object> map;
/**
- * This test hook is used to force the conditions for defect 48182. This
hook is used by
- * Bug48182JUnitTest.
+ * This test hook is used to force the conditions during entry destroy. This
hook is used by
+ * DestroyEntryWithConcurrentOperationJUnitTest.
*/
- 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..98e99a8 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 {
+ verifyEntryDestroyWithCacheClose(false);
+ }
+
+ @Test
+ public void testOffHeapRegionEntryDestroyWithCacheClose() throws Exception {
+ verifyEntryDestroyWithCacheClose(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 verifyEntryDestroyWithCacheClose(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]>'].
