geode-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dschnei...@apache.org
Subject geode git commit: GEODE-2240: prevent unsafe concurrent access to ArrayList
Date Fri, 20 Jan 2017 22:17:31 GMT
Repository: geode
Updated Branches:
  refs/heads/develop 85d45e83c -> 4d5877898


GEODE-2240: prevent unsafe concurrent access to ArrayList

Fixed deadlock by adding expiredTombstonesLock.


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

Branch: refs/heads/develop
Commit: 4d58778985b832a7899056a06fb0de0f2995f55e
Parents: 85d45e8
Author: Darrel Schneider <dschneider@pivotal.io>
Authored: Tue Jan 3 17:00:07 2017 -0800
Committer: Darrel Schneider <dschneider@pivotal.io>
Committed: Fri Jan 20 14:15:34 2017 -0800

----------------------------------------------------------------------
 .../geode/internal/cache/TombstoneService.java  | 22 ++++++++++++--------
 1 file changed, 13 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/4d587789/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
index 2840134..ca682bc 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/cache/TombstoneService.java
@@ -406,10 +406,10 @@ public class TombstoneService {
      */
     private final ExecutorService executor;
     /**
-     * tombstones that have expired and are awaiting batch removal. This variable is only
accessed
-     * by the sweeper thread and so is not guarded
+     * tombstones that have expired and are awaiting batch removal.
      */
     private final List<Tombstone> expiredTombstones;
+    private final Object expiredTombstonesLock = new Object();
 
     /**
      * Force batch expiration
@@ -470,7 +470,7 @@ public class TombstoneService {
     protected boolean removeExpiredIf(Predicate<Tombstone> predicate) {
       boolean result = false;
       long removalSize = 0;
-      synchronized (getBlockGCLock()) {
+      synchronized (expiredTombstonesLock) {
         // Iterate in reverse order to optimize lots of removes.
         // Since expiredTombstones is an ArrayList removing from
         // low indexes requires moving everything at a higher index down.
@@ -520,11 +520,13 @@ public class TombstoneService {
           // Update the GC RVV for all of the affected regions.
           // We need to do this so that we can persist the GC RVV before
           // we start removing entries from the map.
-          for (Tombstone t : expiredTombstones) {
-            DistributedRegion tr = (DistributedRegion) t.region;
-            tr.getVersionVector().recordGCVersion(t.getMemberID(), t.getRegionVersion());
-            if (!reapedKeys.containsKey(tr)) {
-              reapedKeys.put(tr, Collections.emptySet());
+          synchronized (expiredTombstonesLock) {
+            for (Tombstone t : expiredTombstones) {
+              DistributedRegion tr = (DistributedRegion) t.region;
+              tr.getVersionVector().recordGCVersion(t.getMemberID(), t.getRegionVersion());
+              if (!reapedKeys.containsKey(tr)) {
+                reapedKeys.put(tr, Collections.emptySet());
+              }
             }
           }
 
@@ -669,7 +671,9 @@ public class TombstoneService {
       if (logger.isTraceEnabled(LogMarker.TOMBSTONE)) {
         logger.trace(LogMarker.TOMBSTONE, "adding expired tombstone {} to batch", tombstone);
       }
-      expiredTombstones.add(tombstone);
+      synchronized (expiredTombstonesLock) {
+        expiredTombstones.add(tombstone);
+      }
     }
 
     @Override


Mime
View raw message