Author: mreutegg
Date: Mon Feb 1 12:47:32 2016
New Revision: 1727929
URL: http://svn.apache.org/viewvc?rev=1727929&view=rev
Log:
OAK-3949: Deadlock with bulk acquire of TreeLocks
Applied Tomek Rekawek's patch.
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java
(with props)
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java
(with props)
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java?rev=1727929&r1=1727928&r2=1727929&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkLock.java
Mon Feb 1 12:47:32 2016
@@ -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<Lock> locks;
- public BulkLock(List<Lock> locks) {
- this.locks = locks;
+ public BulkLock(Iterable<Lock> locks) {
+ this.locks = Lists.newArrayList(locks);
}
@Override
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java?rev=1727929&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java
(added)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java
Mon Feb 1 12:47:32 2016
@@ -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<ReadWriteLock> locks;
+
+ public BulkReadWriteLock(Iterable<ReadWriteLock> locks) {
+ this.locks = locks;
+ }
+
+ @Override
+ public Lock readLock() {
+ return new BulkLock(Iterables.transform(locks, new
Function<ReadWriteLock, Lock>() {
+ @Override
+ public Lock apply(ReadWriteLock input) {
+ return input.readLock();
+ }
+ }));
+ }
+
+ @Override
+ public Lock writeLock() {
+ return new BulkLock(Iterables.transform(locks, new
Function<ReadWriteLock, Lock>() {
+ @Override
+ public Lock apply(ReadWriteLock input) {
+ return input.writeLock();
+ }
+ }));
+ }
+
+}
Propchange:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/BulkReadWriteLock.java
------------------------------------------------------------------------------
svn:eol-style = native
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java?rev=1727929&r1=1727928&r2=1727929&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentLocks.java
Mon Feb 1 12:47:32 2016
@@ -18,10 +18,7 @@ package org.apache.jackrabbit.oak.plugin
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;
@@ -78,9 +75,8 @@ public class TreeNodeDocumentLocks imple
/**
* This implementation creates two sequences of locks (for the keys and for
* the their parents) using {@link #locks} and {@link #parentLocks}. Then
- * both sequences are zipped into pairs (parentLock, lock) and passed to
the
- * {@link TreeLock#shared(ReadWriteLock, Lock)}. After that all tree locks
- * are acquired.
+ * all parent locks are acquired first and in a second step the locks for
+ * the actual keys.
* <p>
* Since we only acquire a parentLock.read, there's no danger of
* deadlock caused by interleaving locks from two different stripes by two
@@ -99,14 +95,11 @@ public class TreeNodeDocumentLocks imple
return getParentId(keys);
}
});
- Iterator<Lock> lockIt = locks.bulkGet(keys).iterator();
- Iterator<ReadWriteLock> parentLockIt =
parentLocks.bulkGet(parentKeys).iterator();
- List<Lock> acquired = new ArrayList<Lock>(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;
}
Added:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java?rev=1727929&view=auto
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java
(added)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java
Mon Feb 1 12:47:32 2016
@@ -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<Thread> threads = new ArrayList<Thread>();
+ for (int i = 0; i < threadCount; i++) {
+ final List<String> keys = new ArrayList<String>(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
Propchange:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/document/locks/TreeNodeDocumentsLocksTest.java
------------------------------------------------------------------------------
svn:eol-style = native
