Author: baedke Date: Wed Aug 24 18:08:10 2016 New Revision: 1757557 URL: http://svn.apache.org/viewvc?rev=1757557&view=rev Log: OAK-4682: ConcurrentModificationException in JournalEntry.TreeNode
Merging fix into branch 1.4. Modified: jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java Modified: jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java?rev=1757557&r1=1757556&r2=1757557&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/DocumentNodeStore.java Wed Aug 24 18:08:10 2016 @@ -393,7 +393,6 @@ public final class DocumentNodeStore if (builder.getLogging()) { s = new LoggingDocumentStoreWrapper(s); } - this.changes = Collection.JOURNAL.newDocument(s); this.executor = builder.getExecutor(); this.clock = builder.getClock(); @@ -410,6 +409,7 @@ public final class DocumentNodeStore clusterNodeInfo.setLeaseFailureHandler(builder.getLeaseFailureHandler()); } this.store = s; + this.changes = newJournalEntry(); this.clusterId = cid; this.branches = new UnmergedBranches(); this.asyncDelay = builder.getAsyncDelay(); @@ -744,6 +744,16 @@ public final class DocumentNodeStore return diffCache.getStats(); } + /** + * Returns the journal entry that will be stored in the journal with the + * next background updated. + * + * @return the current journal entry. + */ + JournalEntry getCurrentJournalEntry() { + return changes; + } + void invalidateDocChildrenCache() { docChildrenCache.invalidateAll(); } @@ -2024,7 +2034,7 @@ public final class DocumentNodeStore public void acquiring(Revision mostRecent) { if (store.create(JOURNAL, singletonList(changes.asUpdateOp(mostRecent)))) { // success: start with a new document - changes = JOURNAL.newDocument(getDocumentStore()); + changes = newJournalEntry(); } else { // fail: log and keep the changes LOG.error("Failed to write to journal, accumulating changes for future write (~" + changes.getMemory() @@ -2036,6 +2046,10 @@ public final class DocumentNodeStore //-----------------------------< internal >--------------------------------- + private JournalEntry newJournalEntry() { + return new JournalEntry(store, true); + } + /** * Performs an initial read of the _lastRevs on the root document and sets * the root state. Modified: jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java?rev=1757557&r1=1757556&r2=1757557&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/JournalEntry.java Wed Aug 24 18:08:10 2016 @@ -78,8 +78,15 @@ public final class JournalEntry extends private volatile TreeNode changes = null; + private boolean concurrent; + JournalEntry(DocumentStore store) { + this(store, false); + } + + JournalEntry(DocumentStore store, boolean concurrent) { this.store = store; + this.concurrent = concurrent; } static StringSort newSorter() { @@ -364,7 +371,7 @@ public final class JournalEntry extends @Nonnull private TreeNode getChanges() { if (changes == null) { - TreeNode node = new TreeNode(); + TreeNode node = new TreeNode(concurrent); String c = (String) get(CHANGES); if (c != null) { node.parse(new JsopTokenizer(c)); @@ -380,17 +387,22 @@ public final class JournalEntry extends private Map<String, TreeNode> children = NO_CHILDREN; + private final MapFactory mapFactory; private final TreeNode parent; private final String name; TreeNode() { - this(null, ""); + this(false); + } + + TreeNode(boolean concurrent) { + this(concurrent ? MapFactory.CONCURRENT : MapFactory.DEFAULT, null, ""); } - TreeNode(TreeNode parent, String name) { + TreeNode(MapFactory mapFactory, TreeNode parent, String name) { checkArgument(!name.contains("/"), "name must not contain '/': {}", name); - + this.mapFactory = mapFactory; this.parent = parent; this.name = name; } @@ -491,11 +503,11 @@ public final class JournalEntry extends @Nonnull private TreeNode getOrCreate(String name) { if (children == NO_CHILDREN) { - children = Maps.newHashMap(); + children = mapFactory.newMap(); } TreeNode c = children.get(name); if (c == null) { - c = new TreeNode(this, name); + c = new TreeNode(mapFactory, this, name); children.put(name, c); } return c; @@ -507,4 +519,23 @@ public final class JournalEntry extends void node(TreeNode node, String path) throws IOException; } + private interface MapFactory { + + MapFactory DEFAULT = new MapFactory() { + @Override + public Map<String, TreeNode> newMap() { + return Maps.newHashMap(); + } + }; + + MapFactory CONCURRENT = new MapFactory() { + @Override + public Map<String, TreeNode> newMap() { + return Maps.newConcurrentMap(); + } + }; + + Map<String, TreeNode> newMap(); + } + } Modified: jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java?rev=1757557&r1=1757556&r2=1757557&view=diff ============================================================================== --- jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java (original) +++ jackrabbit/oak/branches/1.4/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/JournalEntryTest.java Wed Aug 24 18:08:10 2016 @@ -213,4 +213,32 @@ public class JournalEntryTest { assertTrue(loaderCalled.get()); } } + + // OAK-4682 + @Test + public void concurrentModification() throws Exception { + DocumentNodeStore store = new DocumentMK.Builder().getNodeStore(); + try { + final JournalEntry entry = store.getCurrentJournalEntry(); + Thread t = new Thread(new Runnable() { + @Override + public void run() { + for (int i = 0; i < 100000; i++) { + entry.modified("/node-" + i); + } + } + }); + t.start(); + StringSort sort = JournalEntry.newSorter(); + try { + entry.addTo(sort); + } finally { + sort.close(); + } + t.join(); + } finally { + store.dispose(); + } + } + }