svn commit: r1694208 - /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java

[md...@apache.org]

Author: mduerig
Date: Wed Aug  5 13:25:32 2015
New Revision: 1694208

URL: http://svn.apache.org/r1694208
Log:
OAK-3179: Deadlock between persisted compaction map and cleanup
Move CompactionMap.remove() call outside the synchronized block

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java?rev=1694208&r1=1694207&r2=1694208&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/segment/file/FileStore.java Wed Aug  5 13:25:32 2015
@@ -653,44 +653,49 @@ public class FileStore implements Segmen
      * A new generation of a tar file is created (and segments are only
      * discarded) if doing so releases more than 25% of the space in a tar file.
      */
-    public synchronized void cleanup() throws IOException {
+    public void cleanup() throws IOException {
         Stopwatch watch = Stopwatch.createStarted();
         long initialSize = size();
-        gcMonitor.info("TarMK revision cleanup started. Current repository size {}",
-                humanReadableByteCount(initialSize));
+        CompactionMap cm = tracker.getCompactionMap();
+        Set<UUID> cleanedIds = newHashSet();
 
-        newWriter();
-        tracker.clearCache();
+        synchronized (this) {
+            gcMonitor.info("TarMK revision cleanup started. Current repository size {}",
+                    humanReadableByteCount(initialSize));
 
-        // Suggest to the JVM that now would be a good time
-        // to clear stale weak references in the SegmentTracker
-        System.gc();
-
-        Set<UUID> ids = newHashSet();
-        for (SegmentId id : tracker.getReferencedSegmentIds()) {
-            ids.add(new UUID(
-                    id.getMostSignificantBits(),
-                    id.getLeastSignificantBits()));
-        }
-        writer.collectReferences(ids);
+            newWriter();
+            tracker.clearCache();
 
-        CompactionMap cm = tracker.getCompactionMap();
-        List<TarReader> list = newArrayListWithCapacity(readers.size());
-        Set<UUID> cleanedIds = newHashSet();
-        for (TarReader reader : readers) {
-            TarReader cleaned = reader.cleanup(ids, cm, cleanedIds);
-            if (cleaned == reader) {
-                list.add(reader);
-            } else {
-                if (cleaned != null) {
-                    list.add(cleaned);
+            // Suggest to the JVM that now would be a good time
+            // to clear stale weak references in the SegmentTracker
+            System.gc();
+
+            Set<UUID> ids = newHashSet();
+            for (SegmentId id : tracker.getReferencedSegmentIds()) {
+                ids.add(new UUID(
+                        id.getMostSignificantBits(),
+                        id.getLeastSignificantBits()));
+            }
+            writer.collectReferences(ids);
+
+            List<TarReader> list = newArrayListWithCapacity(readers.size());
+            for (TarReader reader : readers) {
+                TarReader cleaned = reader.cleanup(ids, cm, cleanedIds);
+                if (cleaned == reader) {
+                    list.add(reader);
+                } else {
+                    if (cleaned != null) {
+                        list.add(cleaned);
+                    }
+                    File file = reader.close();
+                    gcMonitor.info("TarMK revision cleanup reclaiming {}", file.getName());
+                    toBeRemoved.addLast(file);
                 }
-                File file = reader.close();
-                gcMonitor.info("TarMK revision cleanup reclaiming {}", file.getName());
-                toBeRemoved.addLast(file);
             }
+            readers = list;
         }
-        readers = list;
+
+        // Do this outside sync to avoid deadlock with SegmentId.getSegment(). See OAK-3179
         cm.remove(cleanedIds);
         long finalSize = size();
         gcMonitor.cleaned(initialSize - finalSize, finalSize);