jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1065599 - /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/
Date Mon, 31 Jan 2011 13:21:12 GMT
Author: jukka
Date: Mon Jan 31 13:21:12 2011
New Revision: 1065599

URL: http://svn.apache.org/viewvc?rev=1065599&view=rev
Log:
JCR-2865: a dead lock in DefaultISMLocking

Use an older 2.1 version of the DefaultISMLocking as a separate VersioningLock class in the
version manager.

This decouples the read-write lock functionality in the version manager from the more performance-critical
ISM locking and fixes the accidental deadlocks caused by the ISM locking optimizations.

Added:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersioningLock.java
Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerBase.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalXAVersionManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImplBase.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionImpl.java?rev=1065599&r1=1065598&r2=1065599&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionImpl.java
Mon Jan 31 13:21:12 2011
@@ -17,7 +17,6 @@
 package org.apache.jackrabbit.core.version;
 
 import org.apache.jackrabbit.core.state.ChildNodeEntry;
-import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
 import org.apache.jackrabbit.core.value.InternalValue;
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.spi.Name;
@@ -148,7 +147,7 @@ class InternalVersionImpl extends Intern
      * {@inheritDoc}
      */
     public List<InternalVersion> getSuccessors() {
-        ReadLock lock = vMgr.acquireReadLock();
+        VersioningLock.ReadLock lock = vMgr.acquireReadLock();
         try {
             InternalValue[] values =
                 node.getPropertyValues(NameConstants.JCR_SUCCESSORS);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerBase.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerBase.java?rev=1065599&r1=1065598&r2=1065599&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerBase.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerBase.java
Mon Jan 31 13:21:12 2011
@@ -31,9 +31,6 @@ import javax.jcr.version.VersionExceptio
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.id.NodeIdFactory;
 import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
-import org.apache.jackrabbit.core.state.DefaultISMLocking;
-import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
-import org.apache.jackrabbit.core.state.ISMLocking.WriteLock;
 import org.apache.jackrabbit.core.state.ItemStateException;
 import org.apache.jackrabbit.core.state.LocalItemStateManager;
 import org.apache.jackrabbit.core.state.NodeReferences;
@@ -75,7 +72,7 @@ abstract class InternalVersionManagerBas
     /**
      * the lock on this version manager
      */
-    private final DefaultISMLocking rwLock = new DefaultISMLocking();
+    private final VersioningLock rwLock = new VersioningLock();
 
     private final NodeIdFactory nodeIdFactory;
 
@@ -144,7 +141,7 @@ abstract class InternalVersionManagerBas
      */
     public InternalVersionHistory getVersionHistoryOfNode(NodeId id)
             throws RepositoryException {
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             String uuid = id.toString();
             Name name = getName(uuid);
@@ -181,10 +178,10 @@ abstract class InternalVersionManagerBas
      * Acquires the write lock on this version manager.
      * @return returns the write lock
      */
-    protected WriteLock acquireWriteLock() {
+    protected VersioningLock.WriteLock acquireWriteLock() {
         while (true) {
             try {
-                return rwLock.acquireWriteLock(null);
+                return rwLock.acquireWriteLock();
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -195,10 +192,10 @@ abstract class InternalVersionManagerBas
      * acquires the read lock on this version manager.
      * @return returns the read lock
      */
-    protected ReadLock acquireReadLock() {
+    protected VersioningLock.ReadLock acquireReadLock() {
         while (true) {
             try {
-                return rwLock.acquireReadLock(null);
+                return rwLock.acquireReadLock();
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -230,9 +227,9 @@ abstract class InternalVersionManagerBas
          */
         private boolean success = false;
 
-        private final WriteLock lock;
+        private final VersioningLock.WriteLock lock;
 
-        public WriteOperation(WriteLock lock) {
+        public WriteOperation(VersioningLock.WriteLock lock) {
             this.lock = lock;
         }
 
@@ -288,7 +285,7 @@ abstract class InternalVersionManagerBas
      */
     private WriteOperation startWriteOperation() throws RepositoryException {
         boolean success = false;
-        WriteLock lock = acquireWriteLock();
+        VersioningLock.WriteLock lock = acquireWriteLock();
         try {
             stateMgr.edit();
             success = true;
@@ -310,7 +307,7 @@ abstract class InternalVersionManagerBas
             throws RepositoryException {
         VersionHistoryInfo info = null;
 
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             String uuid = node.getNodeId().toString();
             Name name = getName(uuid);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java?rev=1065599&r1=1065598&r2=1065599&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
Mon Jan 31 13:21:12 2011
@@ -45,7 +45,6 @@ import org.apache.jackrabbit.core.observ
 import org.apache.jackrabbit.core.persistence.PersistenceManager;
 import org.apache.jackrabbit.core.state.ChangeLog;
 import org.apache.jackrabbit.core.state.ISMLocking;
-import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
 import org.apache.jackrabbit.core.state.ItemState;
 import org.apache.jackrabbit.core.state.ItemStateCacheFactory;
 import org.apache.jackrabbit.core.state.ItemStateException;
@@ -303,7 +302,7 @@ public class InternalVersionManagerImpl 
      * {@inheritDoc}
      */
     public boolean hasItem(NodeId id) {
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             return stateMgr.hasItemState(id);
         } finally {
@@ -323,7 +322,7 @@ public class InternalVersionManagerImpl 
         if (id.equals(activitiesId)) {
             return null;
         }
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             synchronized (versionItems) {
                 InternalVersionItem item = versionItems.get(id);
@@ -466,7 +465,7 @@ public class InternalVersionManagerImpl 
      * @param items items updated
      */
     public void itemsUpdated(Collection<InternalVersionItem> items) {
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             synchronized (versionItems) {
                 for (InternalVersionItem item : items) {
@@ -504,7 +503,7 @@ public class InternalVersionManagerImpl 
      */
     protected void itemDiscarded(InternalVersionItem item) {
         // evict removed item from cache
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             versionItems.remove(item.getId());
         } finally {
@@ -603,7 +602,7 @@ public class InternalVersionManagerImpl 
      */
     public void stateDestroyed(ItemState destroyed) {
         // evict removed item from cache
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             versionItems.remove(destroyed.getId());
         } finally {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalXAVersionManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalXAVersionManager.java?rev=1065599&r1=1065598&r2=1065599&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalXAVersionManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalXAVersionManager.java
Mon Jan 31 13:21:12 2011
@@ -34,8 +34,6 @@ import org.apache.jackrabbit.core.nodety
 import org.apache.jackrabbit.core.observation.EventStateCollection;
 import org.apache.jackrabbit.core.observation.EventStateCollectionFactory;
 import org.apache.jackrabbit.core.state.ChangeLog;
-import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
-import org.apache.jackrabbit.core.state.ISMLocking.WriteLock;
 import org.apache.jackrabbit.core.state.ItemState;
 import org.apache.jackrabbit.core.state.ItemStateCacheFactory;
 import org.apache.jackrabbit.core.state.ItemStateException;
@@ -91,7 +89,7 @@ public class InternalXAVersionManager ex
     /**
      * The global write lock on the version manager.
      */
-    private WriteLock vmgrLock;
+    private VersioningLock.WriteLock vmgrLock;
 
     /**
      * Persistent root node of the version histories.
@@ -718,7 +716,7 @@ public class InternalXAVersionManager ex
      */
     private InternalVersionHistoryImpl makeLocalCopy(InternalVersionHistoryImpl history)
             throws RepositoryException {
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             NodeState state = (NodeState) stateMgr.getItemState(history.getId());
             NodeStateEx stateEx = new NodeStateEx(stateMgr, ntReg, state, null);
@@ -740,7 +738,7 @@ public class InternalXAVersionManager ex
      */
     private InternalActivityImpl makeLocalCopy(InternalActivityImpl act)
             throws RepositoryException {
-        ReadLock lock = acquireReadLock();
+        VersioningLock.ReadLock lock = acquireReadLock();
         try {
             NodeState state = (NodeState) stateMgr.getItemState(act.getId());
             NodeStateEx stateEx = new NodeStateEx(stateMgr, ntReg, state, null);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImplBase.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImplBase.java?rev=1065599&r1=1065598&r2=1065599&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImplBase.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImplBase.java
Mon Jan 31 13:21:12 2011
@@ -35,8 +35,6 @@ import org.apache.jackrabbit.core.value.
 import org.apache.jackrabbit.core.id.NodeId;
 import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
 import org.apache.jackrabbit.core.session.SessionContext;
-import org.apache.jackrabbit.core.state.DefaultISMLocking;
-import org.apache.jackrabbit.core.state.ISMLocking;
 import org.apache.jackrabbit.core.state.ItemStateException;
 import org.apache.jackrabbit.core.state.LocalItemStateManager;
 import org.apache.jackrabbit.core.state.UpdatableItemStateManager;
@@ -93,7 +91,7 @@ abstract public class VersionManagerImpl
     /**
      * the lock on this version manager
      */
-    private final DefaultISMLocking rwLock = new DefaultISMLocking();
+    private final VersioningLock rwLock = new VersioningLock();
 
     /**
      * the node id of the current activity
@@ -456,9 +454,9 @@ abstract public class VersionManagerImpl
          */
         private boolean success = false;
 
-        private final ISMLocking.WriteLock lock;
+        private final VersioningLock.WriteLock lock;
 
-        public WriteOperation(ISMLocking.WriteLock lock) {
+        public WriteOperation(VersioningLock.WriteLock lock) {
             this.lock = lock;
         }
 
@@ -494,10 +492,10 @@ abstract public class VersionManagerImpl
      * Acquires the write lock on this version manager.
      * @return returns the write lock
      */
-    protected ISMLocking.WriteLock acquireWriteLock() {
+    protected VersioningLock.WriteLock acquireWriteLock() {
         while (true) {
             try {
-                return rwLock.acquireWriteLock(null);
+                return rwLock.acquireWriteLock();
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -508,10 +506,10 @@ abstract public class VersionManagerImpl
      * acquires the read lock on this version manager.
      * @return returns the read lock
      */
-    protected ISMLocking.ReadLock acquireReadLock() {
+    protected VersioningLock.ReadLock acquireReadLock() {
         while (true) {
             try {
-                return rwLock.acquireReadLock(null);
+                return rwLock.acquireReadLock();
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -543,7 +541,7 @@ abstract public class VersionManagerImpl
      */
     public WriteOperation startWriteOperation() throws RepositoryException {
         boolean success = false;
-        ISMLocking.WriteLock lock = acquireWriteLock();
+        VersioningLock.WriteLock lock = acquireWriteLock();
         try {
             stateMgr.edit();
             success = true;

Added: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersioningLock.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersioningLock.java?rev=1065599&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersioningLock.java
(added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersioningLock.java
Mon Jan 31 13:21:12 2011
@@ -0,0 +1,219 @@
+/*
+ * 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.core.version;
+
+import java.util.Arrays;
+
+import javax.transaction.xa.Xid;
+
+import org.apache.jackrabbit.core.TransactionContext;
+
+import EDU.oswego.cs.dl.util.concurrent.ReadWriteLock;
+import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock;
+import EDU.oswego.cs.dl.util.concurrent.Sync;
+
+/**
+ * A reentrant read-write lock used by the internal version manager for
+ * synchronization. Unlike a normal reentrant lock, this one allows the lock
+ * to be re-entered not just by a thread that's already holding the lock but
+ * by any thread within the same transaction.
+ */
+class VersioningLock {
+
+    /**
+     * The internal read-write lock.
+     * Thread concerning ReentrantWriterPreferenceReadWriteLock
+     */
+    private final ReadWriteLock rwLock =
+        new ReentrantWriterPreferenceReadWriteLock();
+
+    /**
+     * The internal Xid aware read-write lock.
+     */
+    private final ReadWriteLock xidRwLock = new XidRWLock();
+
+    public ReadLock acquireReadLock() throws InterruptedException {
+        if (TransactionContext.getCurrentXid() == null) {
+            return new ReadLock(rwLock.readLock());
+        } else {
+            return new ReadLock(xidRwLock.readLock());
+        }
+    }
+
+    public WriteLock acquireWriteLock() throws InterruptedException {
+        if (TransactionContext.getCurrentXid() == null) {
+            return new WriteLock(rwLock);
+        } else {
+            return new WriteLock(xidRwLock);
+        }
+    }
+
+    public static class WriteLock {
+
+        private ReadWriteLock readWriteLock;
+
+        private WriteLock(ReadWriteLock readWriteLock)
+                throws InterruptedException {
+            this.readWriteLock = readWriteLock;
+            this.readWriteLock.writeLock().acquire();
+        }
+
+        public void release() {
+            readWriteLock.writeLock().release();
+        }
+
+        public ReadLock downgrade() throws InterruptedException {
+            ReadLock rLock = new ReadLock(readWriteLock.readLock());
+            release();
+            return rLock;
+        }
+
+    }
+
+    public static class ReadLock {
+
+        private final Sync readLock;
+
+        private ReadLock(Sync readLock) throws InterruptedException {
+            this.readLock = readLock;
+            this.readLock.acquire();
+        }
+
+        public void release() {
+            readLock.release();
+        }
+
+    }
+
+    /**
+     * Xid concerning ReentrantWriterPreferenceReadWriteLock
+     */
+    private static final class XidRWLock
+            extends ReentrantWriterPreferenceReadWriteLock {
+
+        private Xid activeXid;
+
+        /**
+         * Check if the given Xid comes from the same globalTX
+         * @param otherXid
+         * @return true if same globalTX otherwise false
+         */
+        boolean isSameGlobalTx(Xid otherXid) {
+            return (activeXid == otherXid) || Arrays.equals(activeXid.getGlobalTransactionId(),
otherXid.getGlobalTransactionId());
+        }
+
+        /**
+         * Allow reader when there is no active Xid, or current Xid owns
+         * the write lock (reentrant).
+         */
+        protected boolean allowReader() {
+            Xid currentXid = TransactionContext.getCurrentXid();
+            return (activeXid == null && waitingWriters_ == 0) || isSameGlobalTx(currentXid);
+        }
+
+        /**
+         * {@inheritDoc}
+         */  
+        protected synchronized boolean startWrite() {
+            Xid currentXid = TransactionContext.getCurrentXid();
+            if (activeXid != null && isSameGlobalTx(currentXid)) { // already held;
re-acquire
+                ++writeHolds_;
+                return true;
+            } else if (writeHolds_ == 0) {
+                if (activeReaders_ == 0 || (readers_.size() == 1 && readers_.get(currentXid)
!= null)) {
+                    activeXid = currentXid;
+                    writeHolds_ = 1;
+                    return true;
+                } else {
+                    return false;
+                }
+            } else {
+                return false;
+            }
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        protected synchronized Signaller endWrite() {
+            --writeHolds_;
+            if (writeHolds_ > 0) {  // still being held
+                return null;
+            } else {
+                activeXid = null;
+                if (waitingReaders_ > 0 && allowReader()) {
+                    return readerLock_;
+                } else if (waitingWriters_ > 0) {
+                    return writerLock_;
+                } else {
+                    return null;
+                }
+            }
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        @SuppressWarnings("unchecked")
+        protected synchronized boolean startRead() {
+            Xid currentXid = TransactionContext.getCurrentXid();
+            Object c = readers_.get(currentXid);
+            if (c != null) { // already held -- just increment hold count
+                readers_.put(currentXid, new Integer(((Integer)(c)).intValue()+1));
+                ++activeReaders_;
+                return true;
+            } else if (allowReader()) {
+                readers_.put(currentXid, IONE);
+                ++activeReaders_;
+                return true;
+            } else {
+                return false;
+            }
+        }
+
+        /**
+         * {@inheritDoc}
+         */
+        @SuppressWarnings("unchecked")
+        protected synchronized Signaller endRead() {
+            Xid currentXid = TransactionContext.getCurrentXid();
+            Object c = readers_.get(currentXid);
+            if (c == null) {
+                throw new IllegalStateException();
+            }
+            --activeReaders_;
+            if (c != IONE) { // more than one hold; decrement count
+                int h = ((Integer)(c)).intValue()-1;
+                Integer ih = (h == 1)? IONE : new Integer(h);
+                readers_.put(currentXid, ih);
+                return null;
+            } else {
+                readers_.remove(currentXid);
+
+                if (writeHolds_ > 0) { // a write lock is still held
+                    return null;
+                } else if (activeReaders_ == 0 && waitingWriters_ > 0) {
+                    return writerLock_;
+                } else  {
+                    return null;
+                }
+            }
+        }
+
+    }
+
+}
\ No newline at end of file



Mime
View raw message