Repository: geode Updated Branches: refs/heads/develop 4018543ac -> a0716a57b
GEODE-2616: fix leak in colocated region removal postDestroyRegion now removes itself from colocated parent colocatedByList Project: http://git-wip-us.apache.org/repos/asf/geode/repo Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/a0716a57 Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/a0716a57 Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/a0716a57 Branch: refs/heads/develop Commit: a0716a57b19f330c991d8bc8d77a8b7b4352ed86 Parents: 4018543 Author: Darrel Schneider <[email protected]> Authored: Fri Jan 20 16:05:41 2017 -0800 Committer: Darrel Schneider <[email protected]> Committed: Wed Mar 8 13:26:41 2017 -0800 ---------------------------------------------------------------------- .../cache/query/internal/DefaultQuery.java | 4 +- .../geode/internal/cache/PartitionedRegion.java | 12 ++--- .../internal/cache/ColocatedPRJUnitTest.java | 47 ++++++++++++++++++++ .../cache/PartitionedRegionTestHelper.java | 11 ++++- 4 files changed, 65 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/geode/blob/a0716a57/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java index 8bfd4fa..a721091 100644 --- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java +++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java @@ -650,8 +650,8 @@ public class DefaultQuery implements Query { continue; } other = allPRs; - if ((((PartitionedRegion) eachPR).colocatedByList.contains(allPRs) - || ((PartitionedRegion) allPRs).colocatedByList.contains(eachPR))) { + if ((((PartitionedRegion) eachPR).getColocatedByList().contains(allPRs) + || ((PartitionedRegion) allPRs).getColocatedByList().contains(eachPR))) { colocated = true; break; } http://git-wip-us.apache.org/repos/asf/geode/blob/a0716a57/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java ---------------------------------------------------------------------- diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java index 9374d4b..173f35c 100755 --- a/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java +++ b/geode-core/src/main/java/org/apache/geode/internal/cache/PartitionedRegion.java @@ -600,7 +600,8 @@ public class PartitionedRegion extends LocalRegion private byte fixedPASet; - public List<PartitionedRegion> colocatedByList = new CopyOnWriteArrayList<PartitionedRegion>(); + private final List<PartitionedRegion> colocatedByList = + new CopyOnWriteArrayList<PartitionedRegion>(); private final PartitionListener[] partitionListeners; @@ -670,9 +671,7 @@ public class PartitionedRegion extends LocalRegion this.colocatedWithRegion = ColocationHelper.getColocatedRegion(this); if (colocatedWithRegion != null) { - synchronized (colocatedWithRegion.colocatedByList) { - colocatedWithRegion.colocatedByList.add(this); - } + colocatedWithRegion.getColocatedByList().add(this); } if (colocatedWithRegion != null && !internalRegionArgs.isUsedForParallelGatewaySenderQueue()) { @@ -786,7 +785,7 @@ public class PartitionedRegion extends LocalRegion return parallelGatewaySenderIds; } - List<PartitionedRegion> getColocatedByList() { + public List<PartitionedRegion> getColocatedByList() { return this.colocatedByList; } @@ -7922,6 +7921,9 @@ public class PartitionedRegion extends LocalRegion } } } + if (colocatedWithRegion != null) { + colocatedWithRegion.getColocatedByList().remove(this); + } RegionLogger.logDestroy(getName(), cache.getMyId(), null, op.isClose()); } http://git-wip-us.apache.org/repos/asf/geode/blob/a0716a57/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java new file mode 100644 index 0000000..66484d9 --- /dev/null +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/ColocatedPRJUnitTest.java @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more contributor license + * agreements. See the NOTICE file distributed with this work for additional information regarding + * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance with the License. You may obtain a + * copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under the License + * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express + * or implied. See the License for the specific language governing permissions and limitations under + * the License. + */ +package org.apache.geode.internal.cache; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertFalse; + +import java.util.List; + +import org.junit.Test; +import org.junit.experimental.categories.Category; + +import org.apache.geode.cache.PartitionAttributes; +import org.apache.geode.cache.PartitionAttributesFactory; +import org.apache.geode.test.junit.categories.IntegrationTest; + +@Category(IntegrationTest.class) +public class ColocatedPRJUnitTest { + @SuppressWarnings("rawtypes") + @Test + public void destroyColocatedPRCheckForLeak() { + PartitionedRegion parent = + (PartitionedRegion) PartitionedRegionTestHelper.createPartionedRegion("PARENT"); + List<PartitionedRegion> colocatedList = parent.getColocatedByList(); + assertEquals(0, colocatedList.size()); + PartitionAttributes PRatts = + new PartitionAttributesFactory().setColocatedWith("/PARENT").create(); + PartitionedRegion child = + (PartitionedRegion) PartitionedRegionTestHelper.createPartionedRegion("CHILD", PRatts); + assertTrue(colocatedList.contains(child)); + child.destroyRegion(); + assertFalse(colocatedList.contains(child)); + } +} http://git-wip-us.apache.org/repos/asf/geode/blob/a0716a57/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java ---------------------------------------------------------------------- diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java index 42c48ac..2824d74 100644 --- a/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java +++ b/geode-core/src/test/java/org/apache/geode/internal/cache/PartitionedRegionTestHelper.java @@ -49,9 +49,16 @@ public class PartitionedRegionTestHelper */ public static Region createPartionedRegion(String regionname) throws RegionExistsException { + return createPartionedRegion(regionname, new PartitionAttributesFactory().create()); + } + + /** + * This method creates a partitioned region with the given PR attributes. The cache created is a + * loner, so this is only suitable for single VM tests. + */ + public static Region createPartionedRegion(String regionname, PartitionAttributes prattribs) + throws RegionExistsException { AttributesFactory attribFactory = new AttributesFactory(); - PartitionAttributesFactory paf = new PartitionAttributesFactory(); - PartitionAttributes prattribs = paf.create(); attribFactory.setDataPolicy(DataPolicy.PARTITION); attribFactory.setPartitionAttributes(prattribs); RegionAttributes regionAttribs = attribFactory.create();
