You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by kl...@apache.org on 2019/08/02 22:12:00 UTC
[geode] branch develop updated (a429325 -> c2ef3c6)
This is an automated email from the ASF dual-hosted git repository.
klund pushed a change to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git.
from a429325 GEODE-7015: Check region persistent ID before close to avoid hang
new 81f8f9c GEODE-7001: Rename LocalRegionTest to LocalRegionPartialMockTest
new c2ef3c6 GEODE-7001: Prevent potential NPE in LocalRegion
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../apache/geode/internal/cache/LocalRegion.java | 24 +-
.../internal/cache/AbstractRegionJUnitTest.java | 6 +-
...onTest.java => LocalRegionPartialMockTest.java} | 2 +-
.../geode/internal/cache/LocalRegionTest.java | 285 ++++-----------------
4 files changed, 74 insertions(+), 243 deletions(-)
copy geode-core/src/test/java/org/apache/geode/internal/cache/{LocalRegionTest.java => LocalRegionPartialMockTest.java} (99%)
[geode] 02/02: GEODE-7001: Prevent potential NPE in LocalRegion
Posted by kl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
klund pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
commit c2ef3c682c885b4d49aad51edc1c11e80553bdfa
Author: Aaron Lindsey <al...@pivotal.io>
AuthorDate: Fri Aug 2 11:18:01 2019 -0700
GEODE-7001: Prevent potential NPE in LocalRegion
Prevent potential NPE during LocalRegion construction when stat sampling
is enabled. Specifically, if getLocalSize() is invoked by the callback
sampler before the RegionMap inside LocalRegion is created then it will
throw an NPE.
Co-authored-by: Aaron Lindsey <al...@pivotal.io>
Co-authored-by: Kirk Lund <kl...@apache.org>
---
.../apache/geode/internal/cache/LocalRegion.java | 24 +++---
.../internal/cache/AbstractRegionJUnitTest.java | 6 +-
.../geode/internal/cache/LocalRegionTest.java | 92 ++++++++++++++++++++++
3 files changed, 110 insertions(+), 12 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
index 6f3a41d..49ca506 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/LocalRegion.java
@@ -540,7 +540,11 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
InternalDataView internalDataView) throws DiskAccessException {
this(regionName, attrs, parentRegion, cache, internalRegionArgs, internalDataView,
RegionMapFactory::createVM, new DefaultServerRegionProxyConstructor(),
- new DefaultEntryEventFactory(), poolName -> (PoolImpl) PoolManager.find(poolName));
+ new DefaultEntryEventFactory(), poolName -> (PoolImpl) PoolManager.find(poolName),
+ (LocalRegion region) -> new RegionPerfStats(
+ cache.getInternalDistributedSystem().getStatisticsManager(),
+ "RegionStats-" + regionName, cache.getCachePerfStats(),
+ region, cache.getMeterRegistry()));
}
@VisibleForTesting
@@ -548,7 +552,8 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
InternalCache cache, InternalRegionArguments internalRegionArgs,
InternalDataView internalDataView, RegionMapConstructor regionMapConstructor,
ServerRegionProxyConstructor serverRegionProxyConstructor,
- EntryEventFactory entryEventFactory, PoolFinder poolFinder)
+ EntryEventFactory entryEventFactory, PoolFinder poolFinder,
+ java.util.function.Function<LocalRegion, RegionPerfStats> regionPerfStatsFactory)
throws DiskAccessException {
super(cache, attrs, regionName, internalRegionArgs, poolFinder);
@@ -587,6 +592,11 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
}
initializingRegion.set(this);
+ diskStoreImpl = findDiskStore(attrs, internalRegionArgs);
+ diskRegion = createDiskRegion(internalRegionArgs);
+ entries = createRegionMap(internalRegionArgs);
+ subregions = new ConcurrentHashMap();
+
if (internalRegionArgs.getCachePerfStatsHolder() != null) {
hasOwnStats = false;
cachePerfStats = internalRegionArgs.getCachePerfStatsHolder().getCachePerfStats();
@@ -597,18 +607,10 @@ public class LocalRegion extends AbstractRegion implements LoaderHelperFactory,
cachePerfStats = cache.getCachePerfStats();
} else {
hasOwnStats = true;
- cachePerfStats =
- new RegionPerfStats(cache.getInternalDistributedSystem().getStatisticsManager(),
- "RegionStats-" + regionName, cache.getCachePerfStats(),
- this, cache.getMeterRegistry());
+ cachePerfStats = regionPerfStatsFactory.apply(this);
}
}
- diskStoreImpl = findDiskStore(attrs, internalRegionArgs);
- diskRegion = createDiskRegion(internalRegionArgs);
- entries = createRegionMap(internalRegionArgs);
- subregions = new ConcurrentHashMap();
-
// we only need a destroy lock if this is a root
if (parentRegion == null) {
initRoot();
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionJUnitTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionJUnitTest.java
index a04f5b7..09c7c5f 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionJUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/AbstractRegionJUnitTest.java
@@ -20,6 +20,7 @@ import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.when;
import java.util.Set;
+import java.util.function.Function;
import org.junit.Test;
@@ -113,8 +114,11 @@ public class AbstractRegionJUnitTest {
DiskWriteAttributes diskWriteAttributes = mock(DiskWriteAttributes.class);
when(regionAttributes.getDiskWriteAttributes()).thenReturn(diskWriteAttributes);
RegionMapConstructor regionMapConstructor = mock(RegionMapConstructor.class);
+ Function<LocalRegion, RegionPerfStats> regionPerfStatsFactory =
+ (localRegion) -> mock(RegionPerfStats.class);
AbstractRegion region = new LocalRegion("regionName", regionAttributes, null, Fakes.cache(),
- new InternalRegionArguments(), null, regionMapConstructor, null, null, null);
+ new InternalRegionArguments(), null, regionMapConstructor, null, null, null,
+ regionPerfStatsFactory);
return region;
}
}
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
new file mode 100644
index 0000000..22adf2b
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
@@ -0,0 +1,92 @@
+/*
+ * 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.assertj.core.api.Assertions.assertThatCode;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.function.Function;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.cache.DataPolicy;
+import org.apache.geode.cache.DiskWriteAttributes;
+import org.apache.geode.cache.EvictionAction;
+import org.apache.geode.cache.EvictionAttributes;
+import org.apache.geode.cache.ExpirationAttributes;
+import org.apache.geode.cache.RegionAttributes;
+import org.apache.geode.distributed.internal.DSClock;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.cache.AbstractRegion.PoolFinder;
+import org.apache.geode.internal.cache.LocalRegion.RegionMapConstructor;
+import org.apache.geode.internal.cache.LocalRegion.ServerRegionProxyConstructor;
+
+public class LocalRegionTest {
+
+ private RegionAttributes regionAttributes;
+ private InternalCache cache;
+ private InternalRegionArguments internalRegionArguments;
+ private InternalDataView internalDataView;
+ private RegionMapConstructor regionMapConstructor;
+ private ServerRegionProxyConstructor serverRegionProxyConstructor;
+ private EntryEventFactory entryEventFactory;
+ private PoolFinder poolFinder;
+
+ @Before
+ public void setup() {
+ cache = mock(InternalCache.class);
+ entryEventFactory = mock(EntryEventFactory.class);
+ internalDataView = mock(InternalDataView.class);
+ internalRegionArguments = mock(InternalRegionArguments.class);
+ poolFinder = mock(PoolFinder.class);
+ regionAttributes = mock(RegionAttributes.class);
+ regionMapConstructor = mock(RegionMapConstructor.class);
+ serverRegionProxyConstructor = mock(ServerRegionProxyConstructor.class);
+
+ DiskWriteAttributes diskWriteAttributes = mock(DiskWriteAttributes.class);
+ EvictionAttributes evictionAttributes = mock(EvictionAttributes.class);
+ ExpirationAttributes expirationAttributes = mock(ExpirationAttributes.class);
+ InternalDistributedSystem internalDistributedSystem = mock(InternalDistributedSystem.class);
+
+ when(cache.getInternalDistributedSystem()).thenReturn(internalDistributedSystem);
+ when(evictionAttributes.getAction()).thenReturn(EvictionAction.NONE);
+ when(internalDistributedSystem.getClock()).thenReturn(mock(DSClock.class));
+ when(regionAttributes.getDataPolicy()).thenReturn(DataPolicy.DEFAULT);
+ when(regionAttributes.getDiskWriteAttributes()).thenReturn(diskWriteAttributes);
+ when(regionAttributes.getEntryIdleTimeout()).thenReturn(expirationAttributes);
+ when(regionAttributes.getEntryTimeToLive()).thenReturn(expirationAttributes);
+ when(regionAttributes.getEvictionAttributes()).thenReturn(evictionAttributes);
+ when(regionAttributes.getRegionIdleTimeout()).thenReturn(expirationAttributes);
+ when(regionAttributes.getRegionTimeToLive()).thenReturn(expirationAttributes);
+ when(regionMapConstructor.create(any(), any(), any())).thenReturn(mock(RegionMap.class));
+ }
+
+ @Test
+ public void getLocalSizeDoesNotThrowNullPointerExceptionDuringConstruction() {
+ Function<LocalRegion, RegionPerfStats> regionPerfStatsFactory = (localRegion) -> {
+ localRegion.getLocalSize();
+ return mock(RegionPerfStats.class);
+ };
+
+ assertThatCode(() -> new LocalRegion("region", regionAttributes, null, cache,
+ internalRegionArguments, internalDataView, regionMapConstructor,
+ serverRegionProxyConstructor, entryEventFactory, poolFinder, regionPerfStatsFactory))
+ .doesNotThrowAnyException();
+ }
+}
[geode] 01/02: GEODE-7001: Rename LocalRegionTest to
LocalRegionPartialMockTest
Posted by kl...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
klund pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
commit 81f8f9c1ca1ec9a591287905e559051284117057
Author: Aaron Lindsey <al...@pivotal.io>
AuthorDate: Fri Aug 2 11:02:34 2019 -0700
GEODE-7001: Rename LocalRegionTest to LocalRegionPartialMockTest
Co-authored-by: Aaron Lindsey <al...@pivotal.io>
Co-authored-by: Kirk Lund <kl...@apache.org>
---
.../cache/{LocalRegionTest.java => LocalRegionPartialMockTest.java} | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionPartialMockTest.java
similarity index 99%
rename from geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
rename to geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionPartialMockTest.java
index 7a46090..c345d6b 100644
--- a/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionTest.java
+++ b/geode-core/src/test/java/org/apache/geode/internal/cache/LocalRegionPartialMockTest.java
@@ -53,7 +53,7 @@ import org.apache.geode.internal.cache.tier.sockets.CacheServerStats;
import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
import org.apache.geode.internal.cache.tier.sockets.ConnectionListener;
-public class LocalRegionTest {
+public class LocalRegionPartialMockTest {
private LocalRegion region;
private EntryEventImpl event;
private ServerRegionProxy serverRegionProxy;