jchen21 commented on a change in pull request #5039:
URL: https://github.com/apache/geode/pull/5039#discussion_r418682332



##########
File path: 
geode-core/src/test/java/org/apache/geode/internal/cache/GemFireCacheImplCloseTest.java
##########
@@ -190,41 +188,33 @@ public void close_doesNothingIfAlreadyClosed() {
     verify(internalDistributedSystem).disconnect();
   }
 
-  @Ignore("GEODE-8060: wrong thread wins")
   @Test
   public void close_blocksUntilFirstCallToCloseCompletes() throws Exception {
     gemFireCacheImpl = gemFireCacheImpl(false);
     CyclicBarrier cyclicBarrier = new CyclicBarrier(3);
-    AtomicLong winner = new AtomicLong();
 
-    Future<Long> close1 = executorServiceRule.submit(() -> {
+    Future<Boolean> close1 = executorServiceRule.submit(() -> {
       synchronized (GemFireCacheImpl.class) {
-        long threadId = Thread.currentThread().getId();
         cyclicBarrier.await(getTimeout().toMillis(), MILLISECONDS);
-        gemFireCacheImpl.close();
-        winner.compareAndSet(0, threadId);
-        return threadId;
+        return gemFireCacheImpl.doClose("test", null, false, false, false);
       }
     });
 
     await().until(() -> cyclicBarrier.getNumberWaiting() == 1);
 
-    Future<Long> close2 = executorServiceRule.submit(() -> {
-      long threadId = Thread.currentThread().getId();
+    Future<Boolean> close2 = executorServiceRule.submit(() -> {
       cyclicBarrier.await(getTimeout().toMillis(), MILLISECONDS);
-      gemFireCacheImpl.close();
-      winner.compareAndSet(0, threadId);
-      return threadId;
+      return gemFireCacheImpl.doClose("test", null, false, false, false);
     });
 
     cyclicBarrier.await(getTimeout().toMillis(), MILLISECONDS);
 
-    long threadId1 = close1.get();
-    long threadId2 = close2.get();
+    boolean closedCache1 = close1.get();
+    boolean closedCache2 = close2.get();
 
-    assertThat(winner.get())
-        .as("ThreadId1=" + threadId1 + " and threadId2=" + threadId2)
-        .isEqualTo(threadId1);
+    assertThat(closedCache1)

Review comment:
       Just for safe, can we also assert `closedCache2` is `false`?




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to