geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From u..@apache.org
Subject geode git commit: GEODE-3314: Initial commit to reproduce DLockToken leak
Date Tue, 25 Jul 2017 23:46:38 GMT
Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-3314 [created] 4643e25fe


GEODE-3314: Initial commit to reproduce DLockToken leak


Project: http://git-wip-us.apache.org/repos/asf/geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/geode/commit/4643e25f
Tree: http://git-wip-us.apache.org/repos/asf/geode/tree/4643e25f
Diff: http://git-wip-us.apache.org/repos/asf/geode/diff/4643e25f

Branch: refs/heads/feature/GEODE-3314
Commit: 4643e25fe8b4dea7d7160f665529920c3a8789a8
Parents: 09dd45f
Author: Udo Kohlmeyer <ukohlmeyer@pivotal.io>
Authored: Tue Jul 25 16:46:34 2017 -0700
Committer: Udo Kohlmeyer <ukohlmeyer@pivotal.io>
Committed: Tue Jul 25 16:46:34 2017 -0700

----------------------------------------------------------------------
 .../internal/locks/DLockRequestProcessor.java   |   6 -
 .../internal/locks/DLockService.java            |  42 +++---
 .../internal/locks/DLockServiceJUnitTest.java   | 133 +++++++++++++++++++
 3 files changed, 155 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/4643e25f/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
index f0ee31b..3f42adb 100755
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockRequestProcessor.java
@@ -202,18 +202,12 @@ public class DLockRequestProcessor extends ReplyProcessor21 {
     Assert.assertTrue(lockId > -1, "lockId is < 0: " + this);
     this.request.lockId = lockId;
 
-    // setDoneProcessing(false);
-
     // local grantor... don't use messaging... fake it
     if (isLockGrantor()) {
       if (isDebugEnabled_DLS) {
         logger.trace(LogMarker.DLS, "DLockRequestProcessor processing lock request directly");
       }
       this.request.setSender(this.dm.getDistributionManagerId());
-      /*
-       * if (svc.isDestroyed()) { return false; }
-       */
-      // svc.checkDestroyed();
 
       // calls processor (this) process...
       this.request.processLocally(this.dm);

http://git-wip-us.apache.org/repos/asf/geode/blob/4643e25f/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
index c38cdad..0e51bf2 100644
--- a/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
+++ b/geode-core/src/main/java/org/apache/geode/distributed/internal/locks/DLockService.java
@@ -1388,7 +1388,6 @@ public class DLockService extends DistributedLockService {
       throw new InterruptedException();
     }
 
-    boolean abnormalExit = true;
     boolean safeExit = true;
     try { // try-block for abnormalExit and safeExit
 
@@ -1438,23 +1437,7 @@ public class DLockService extends DistributedLockService {
         while (keepTrying) {
           if (DEBUG_LOCK_REQUEST_LOOP) {
             loopCount++;
-            if (loopCount > DEBUG_LOCK_REQUEST_LOOP_COUNT) {
-              Integer count = Integer.valueOf(DEBUG_LOCK_REQUEST_LOOP_COUNT);
-              String s =
-                  LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES
-                      .toLocalizedString(count);
-
-              InternalGemFireError e = new InternalGemFireError(s);
-              logger.error(LogMarker.DLS,
-                  LocalizedMessage.create(
-                      LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES,
-                      count),
-                  e);
-              throw e;
-            }
-            /*
-             * if (loopCount > 1) { Thread.sleep(1000); }
-             */
+            extracted1(loopCount);
           }
 
           checkDestroyed();
@@ -1552,6 +1535,7 @@ public class DLockService extends DistributedLockService {
           } // else: non-reentrant or reentrant w/ non-infinite lease
 
           if (gotLock) {
+            token.incUsage();
             // if (processor != null) (cannot be null)
             { // TODO: can be null after restoring above optimization
               // non-reentrant lock needs to getLeaseExpireTime
@@ -1716,7 +1700,6 @@ public class DLockService extends DistributedLockService {
         logger.trace(LogMarker.DLS, "{}, name: {} - exiting lock() returning {}", this, name,
             gotLock);
       }
-      abnormalExit = false;
       return gotLock;
     } // try-block for abnormalExit and safeExit
 
@@ -1735,6 +1718,26 @@ public class DLockService extends DistributedLockService {
     }
   }
 
+  private void extracted1(int loopCount) {
+    if (loopCount > DEBUG_LOCK_REQUEST_LOOP_COUNT) {
+      Integer count = Integer.valueOf(DEBUG_LOCK_REQUEST_LOOP_COUNT);
+      String s =
+          LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES
+              .toLocalizedString(count);
+
+      InternalGemFireError e = new InternalGemFireError(s);
+      logger.error(LogMarker.DLS,
+          LocalizedMessage.create(
+              LocalizedStrings.DLockService_DEBUG_LOCKINTERRUPTIBLY_HAS_GONE_HOT_AND_LOOPED_0_TIMES,
+              count),
+          e);
+      throw e;
+    }
+            /*
+             * if (loopCount > 1) { Thread.sleep(1000); }
+             */
+  }
+
   /**
    * Allow locking to resume.
    */
@@ -2659,7 +2662,6 @@ public class DLockService extends DistributedLockService {
           }
           getStats().incTokens(1);
         }
-        token.incUsage();
       }
       return token;
     }

http://git-wip-us.apache.org/repos/asf/geode/blob/4643e25f/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceJUnitTest.java
new file mode 100644
index 0000000..9968f09
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/distributed/internal/locks/DLockServiceJUnitTest.java
@@ -0,0 +1,133 @@
+package org.apache.geode.distributed.internal.locks;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.apache.geode.cache.Cache;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.cache.ExpirationAction;
+import org.apache.geode.cache.ExpirationAttributes;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.Scope;
+import org.apache.geode.internal.cache.DistributedRegion;
+import org.apache.geode.test.junit.categories.UnitTest;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.util.Collection;
+import java.util.LinkedList;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+import java.util.concurrent.locks.Lock;
+
+@Category(UnitTest.class)
+public class DLockServiceJUnitTest {
+
+  private Cache cache;
+  private ExecutorService executorService;
+  private DistributedRegion testRegion;
+
+  @Test
+  public void basicDLockUsage() throws InterruptedException, ExecutionException, TimeoutException
{
+    Lock lock = testRegion.getDistributedLock("testLockName");
+    lock.lockInterruptibly();
+
+    Future<Boolean>
+        future =
+        executorService.submit(() -> lock.tryLock());
+    assertFalse("should not be able to get lock from another thread",
+        future.get(5, TimeUnit.SECONDS));
+
+    assertTrue("Lock is reentrant", lock.tryLock());
+    // now locked twice.
+
+    future =
+        executorService.submit(() -> lock.tryLock());
+    assertFalse("should not be able to get lock from another thread",
+        future.get(5, TimeUnit.SECONDS));
+
+    lock.unlock();
+
+    future = executorService.submit(() -> lock.tryLock());
+    assertFalse("should not be able to get lock from another thread", future.get());
+
+    lock.unlock();
+
+    future = executorService.submit(() -> {
+      boolean locked = lock.tryLock();
+      if (!locked) {
+        return false;
+      }
+      lock.unlock();
+      return true;
+    });
+    assertTrue("Another thread can now take out the lock", future.get(5, TimeUnit.SECONDS));
+
+    DLockService lockService = (DLockService) testRegion.getLockService();
+    Collection<DLockToken> tokens = lockService.getTokens();
+
+    assertEquals(1, tokens.size());
+
+    for (DLockToken token : tokens) {
+      assertEquals(0, token.getUsageCount());
+    }
+  }
+
+  @Before
+  public void setUp() {
+    cache = new CacheFactory().create();
+    testRegion = (DistributedRegion) cache.createRegionFactory(RegionShortcut.REPLICATE)
+        .setScope(Scope.GLOBAL)
+        .setEntryTimeToLive(new ExpirationAttributes(1, ExpirationAction.DESTROY))
+        .create("testRegion");
+    testRegion.becomeLockGrantor();
+
+    executorService = Executors.newFixedThreadPool(5);
+  }
+
+  @After
+  public void tearDown() {
+    cache.close();
+    executorService.shutdownNow();
+  }
+
+  @Test
+  public void multipleThreadsWithCache() {
+    LinkedList<Future> futures = new LinkedList<>();
+    for (int i = 0; i < 5; i++) {
+      futures.add(executorService.submit(this::putTestKey));
+    }
+
+    futures.stream().forEach(future -> {
+      try {
+        future.get();
+      } catch (InterruptedException e) {
+        e.printStackTrace();
+      } catch (ExecutionException e) {
+        e.printStackTrace();
+      }
+    });
+
+    DLockService lockService = (DLockService) testRegion.getLockService();
+    Collection<DLockToken> tokens = lockService.getTokens();
+
+    assertEquals(1, tokens.size());
+
+    for (DLockToken token : tokens) {
+      assertEquals(0, token.getUsageCount());
+    }
+  }
+
+  private void putTestKey() {
+    for (int i = 0; i < 1000; i++) {
+      testRegion.put("testKey", "testValue");
+    }
+  }
+}
\ No newline at end of file


Mime
View raw message