You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/03/02 17:41:03 UTC

[GitHub] [geode] kirklund commented on a change in pull request #5925: GEODE-8671: Two threads calling get and retrieve the same PdxInstance, resulting in corruption

kirklund commented on a change in pull request #5925:
URL: https://github.com/apache/geode/pull/5925#discussion_r585774024



##########
File path: geode-core/src/distributedTest/java/org/apache/geode/internal/cache/RegionConcurrentOperationDUnitTest.java
##########
@@ -131,6 +140,49 @@ public void getOnPreLoadedRegionFromMultipleThreadsReturnSameObject() throws Exc
     assertThat(cacheRule.getCache().getRegion(regionName).size()).isEqualTo(1);
   }
 
+  @Test
+  public void getOnPartitionedRegionFromMultipleThreadsReturnsDifferentPdxInstances()
+      throws Exception {
+    String regionName = getClass().getSimpleName();
+    CacheFactory cacheFactory = new CacheFactory();
+    cacheFactory.setPdxReadSerialized(true);
+    cacheRule.createCache(cacheFactory);
+    InternalCache cache = cacheRule.getCache();
+    cache.setCopyOnRead(true);
+    Region region = cache.createRegionFactory(PARTITION)
+        .create(regionName);
+
+    // Keep doing this concurrency test for 30 seconds.
+    long endTime = Duration.ofSeconds(30).toMillis() + System.currentTimeMillis();
+
+    while (System.currentTimeMillis() < endTime) {
+      Callable<Object> getValue = () -> {
+        while (true) {
+          Object value = region.get(key);
+          if (value != null) {
+            return value;
+          }
+        }
+      };
+
+      // In this test, two threads are doing gets. One thread puts the value
+      // We expect that the threads will *always* get different PdxInstance values
+      Future get1 = executorServiceRule.submit(getValue);
+      Future get2 = executorServiceRule.submit(getValue);
+      Future put = executorServiceRule.submit(() -> region.put(key, new TestValue()));

Review comment:
       If the Future is not going to have a result then go with `Future<Void>`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org