diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java index 7fbcf74..be809a7 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java @@ -23,6 +23,8 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; +import com.google.common.collect.Lists; + /** * This class exposes a list of locks as a single Lock instance. */ @@ -30,8 +32,8 @@ class BulkLock implements Lock { private final List locks; - public BulkLock(List locks) { - this.locks = locks; + public BulkLock(Iterable locks) { + this.locks = Lists.newArrayList(locks); } @Override diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java new file mode 100644 index 0000000..eb496ce --- /dev/null +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.plugins.document.locks; + +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReadWriteLock; + +import com.google.common.base.Function; +import com.google.common.collect.Iterables; + +/** + * This class exposes a list of ReadWriteLocks as a single Lock instance. + */ +class BulkReadWriteLock implements ReadWriteLock { + + private Iterable locks; + + public BulkReadWriteLock(Iterable locks) { + this.locks = locks; + } + + @Override + public Lock readLock() { + return new BulkLock(Iterables.transform(locks, new Function() { + @Override + public Lock apply(ReadWriteLock input) { + return input.readLock(); + } + })); + } + + @Override + public Lock writeLock() { + return new BulkLock(Iterables.transform(locks, new Function() { + @Override + public Lock apply(ReadWriteLock input) { + return input.writeLock(); + } + })); + } + +} diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java index 3a75f7b..39474b9 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java @@ -18,10 +18,7 @@ package org.apache.jackrabbit.oak.plugins.document.locks; import static com.google.common.base.Preconditions.checkNotNull; -import java.util.ArrayList; import java.util.Collection; -import java.util.Iterator; -import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.Condition; @@ -99,14 +96,11 @@ public class TreeNodeDocumentLocks implements NodeDocumentLocks { return getParentId(keys); } }); - Iterator lockIt = locks.bulkGet(keys).iterator(); - Iterator parentLockIt = parentLocks.bulkGet(parentKeys).iterator(); - List acquired = new ArrayList(keys.size()); - while (lockIt.hasNext()) { - acquired.add(TreeLock.shared(parentLockIt.next(), lockIt.next())); - } - Lock lock = new BulkLock(acquired); + ReadWriteLock bulkParentLock = new BulkReadWriteLock(parentLocks.bulkGet(parentKeys)); + Lock bulkChildrenLock = new BulkLock(locks.bulkGet(keys)); + + Lock lock = TreeLock.shared(bulkParentLock, bulkChildrenLock); lock.lock(); return lock; } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java new file mode 100644 index 0000000..194076c --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java @@ -0,0 +1,95 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.plugins.document.locks; + +import static org.junit.Assert.fail; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.concurrent.locks.Lock; + +import org.junit.Test; + +public class TreeNodeDocumentsLocksTest { + + /** + * Test for the OAK-3949. It uses multiple threads to acquire a shared lock + * collection. There's also another set of threads that acquires parent + * locks exclusively. Such combination can't lead to a deadlock. + */ + @Test + public void testBulkAcquire() throws InterruptedException { + int threadCount = 10; + int locksCount = 100; + + final TreeNodeDocumentLocks locks = new TreeNodeDocumentLocks(); + List threads = new ArrayList(); + for (int i = 0; i < threadCount; i++) { + final List keys = new ArrayList(locksCount); + for (int j = 0; j < locksCount; j++) { + keys.add(String.format("2:/parent_%d/lock_%d", j, j)); + } + threads.add(new Thread(new Runnable() { + @Override + public void run() { + Lock lock = locks.acquire(keys); + try { + Thread.sleep(100); + } catch (InterruptedException e) { + } finally { + lock.unlock(); + } + } + })); + } + + for (int j = 0; j < threadCount; j++) { + final int from = j * 10; + final int to = (j + 1) * 10; + threads.add(new Thread(new Runnable() { + @Override + public void run() { + try { + for (int i = from; i < to; i++) { + Lock parentLock = locks.acquireExclusive(String.format("1:/parent_%d", i)); + Thread.sleep(100); + Lock childLock = locks.acquire(String.format("2:/parent_%d/lock_%d", i, i)); + Thread.sleep(100); + childLock.unlock(); + Thread.sleep(100); + parentLock.unlock(); + } + } catch (InterruptedException e) { + } + } + })); + } + + Collections.shuffle(threads); + for (Thread t : threads) { + t.start(); + } + + for (Thread t : threads) { + t.join(10000); + if (t.isAlive()) { + fail("Thread hasn't stopped in 10s"); + } + } + } +} \ No newline at end of file