jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r750011 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: state/ version/
Date Wed, 04 Mar 2009 14:08:49 GMT
Author: jukka
Date: Wed Mar  4 14:08:48 2009
New Revision: 750011

URL: http://svn.apache.org/viewvc?rev=750011&view=rev
Log:
JCR-2000: Deadlock on concurrent commits

Use DefaultISMLocking as the versioning lock to get the same JCR-1334 support for versioning
as was already implemented for default workspace locking.

Make all transactions acquire the versioning lock regardless of whether versioning changes
are included in the transaction. This prevents potential deadlocks as even a non-versioning
commit will access the version store for reference checks.

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/AbstractVersionManager.java
    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/VersionManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/XAVersionManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java?rev=750011&r1=750010&r2=750011&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java
Wed Mar  4 14:08:48 2009
@@ -52,8 +52,8 @@
         return new WriteLock() {
 
             {
-                rwLock.setActiveXid(TransactionContext.getCurrentXid());
                 rwLock.writeLock().acquire();
+                rwLock.setActiveXid(TransactionContext.getCurrentXid());
             }
 
             /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/AbstractVersionManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/AbstractVersionManager.java?rev=750011&r1=750010&r2=750011&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/AbstractVersionManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/AbstractVersionManager.java
Wed Mar  4 14:08:48 2009
@@ -16,14 +16,15 @@
  */
 package org.apache.jackrabbit.core.version;
 
-import EDU.oswego.cs.dl.util.concurrent.ReadWriteLock;
-import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock;
 import org.apache.jackrabbit.core.NodeId;
 import org.apache.jackrabbit.core.NodeImpl;
 import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
+import org.apache.jackrabbit.core.state.DefaultISMLocking;
 import org.apache.jackrabbit.core.state.ItemStateException;
 import org.apache.jackrabbit.core.state.LocalItemStateManager;
 import org.apache.jackrabbit.core.state.NodeState;
+import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
+import org.apache.jackrabbit.core.state.ISMLocking.WriteLock;
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.spi.commons.name.NameConstants;
 import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
@@ -66,17 +67,7 @@
     /**
      * the lock on this version manager
      */
-    private final ReadWriteLock rwLock =
-            new ReentrantWriterPreferenceReadWriteLock() {
-                /**
-                 * Allow reader when there is no active writer, or current
-                 * thread owns the write lock (reentrant).
-                 */
-                protected boolean allowReader() {
-                    return activeWriter_ == null
-                        || activeWriter_ == Thread.currentThread();
-                }
-            };
+    private final DefaultISMLocking rwLock = new DefaultISMLocking();
 
     public AbstractVersionManager(NodeTypeRegistry ntReg) {
         this.ntReg = ntReg;
@@ -108,13 +99,12 @@
     //-------------------------------------------------------< implementation >
 
     /**
-     * aquires the write lock on this version manager.
+     * Acquires the write lock on this version manager.
      */
-    protected void acquireWriteLock() {
+    protected WriteLock acquireWriteLock() {
         while (true) {
             try {
-                rwLock.writeLock().acquire();
-                return;
+                return rwLock.acquireWriteLock(null);
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -122,20 +112,12 @@
     }
 
     /**
-     * releases the write lock on this version manager.
-     */
-    protected void releaseWriteLock() {
-        rwLock.writeLock().release();
-    }
-
-    /**
-     * aquires the read lock on this version manager.
+     * acquires the read lock on this version manager.
      */
-    protected void acquireReadLock() {
+    protected ReadLock acquireReadLock() {
         while (true) {
             try {
-                rwLock.readLock().acquire();
-                return;
+                return rwLock.acquireReadLock(null);
             } catch (InterruptedException e) {
                 // ignore
             }
@@ -143,13 +125,6 @@
     }
 
     /**
-     * releases the read lock on this version manager.
-     */
-    protected void releaseReadLock() {
-        rwLock.readLock().release();
-    }
-
-    /**
      * Helper for managing write operations.
      */
     private class WriteOperation {
@@ -159,6 +134,12 @@
          */
         private boolean success = false;
 
+        private final WriteLock lock;
+
+        public WriteOperation(WriteLock lock) {
+            this.lock = lock;
+        }
+
         /**
          * Saves the pending operations in the {@link LocalItemStateManager}.
          *
@@ -182,7 +163,7 @@
                     stateMgr.cancel();
                 }
             } finally {
-                releaseWriteLock();
+                lock.release();
             }
         }
     }
@@ -211,16 +192,16 @@
      */
     private WriteOperation startWriteOperation() throws RepositoryException {
         boolean success = false;
-        acquireWriteLock();
+        WriteLock lock = acquireWriteLock();
         try {
             stateMgr.edit();
             success = true;
-            return new WriteOperation();
+            return new WriteOperation(lock);
         } catch (IllegalStateException e) {
             throw new RepositoryException("Unable to start edit operation.", e);
         } finally {
             if (!success) {
-                releaseWriteLock();
+                lock.release();
             }
         }
     }
@@ -232,7 +213,7 @@
             throws RepositoryException {
         VersionHistoryInfo info = null;
 
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             String uuid = node.getNodeId().getUUID().toString();
             Name name = getName(uuid);
@@ -246,7 +227,7 @@
                         history.getState().getChildNodeEntry(root, 1).getId());
             }
         } finally {
-            releaseReadLock();
+            lock.release();
         }
 
         if (info == null) {

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=750011&r1=750010&r2=750011&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
Wed Mar  4 14:08:48 2009
@@ -17,6 +17,7 @@
 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.NodeId;
 import org.apache.jackrabbit.spi.Name;
@@ -136,7 +137,7 @@
      * {@inheritDoc}
      */
     public InternalVersion[] getSuccessors() {
-        vMgr.acquireReadLock();
+        ReadLock lock = vMgr.acquireReadLock();
         try {
             InternalValue[] values = node.getPropertyValues(NameConstants.JCR_SUCCESSORS);
             if (values != null) {
@@ -150,7 +151,7 @@
                 return new InternalVersion[0];
             }
         } finally {
-            vMgr.releaseReadLock();
+            lock.release();
         }
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImpl.java?rev=750011&r1=750010&r2=750011&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/VersionManagerImpl.java
Wed Mar  4 14:08:48 2009
@@ -42,6 +42,7 @@
 import org.apache.jackrabbit.core.state.SharedItemStateManager;
 import org.apache.jackrabbit.core.state.ISMLocking;
 import org.apache.jackrabbit.core.state.NoSuchItemStateException;
+import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
 import org.apache.jackrabbit.core.value.InternalValue;
 import org.apache.jackrabbit.core.virtual.VirtualItemStateProvider;
 import org.apache.jackrabbit.spi.Path;
@@ -233,11 +234,11 @@
      * {@inheritDoc}
      */
     public boolean hasItem(NodeId id) {
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             return stateMgr.hasItemState(id);
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
 
@@ -250,7 +251,7 @@
         if (id.equals(getHistoryRootId())) {
             return null;
         }
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             synchronized (versionItems) {
                 InternalVersionItem item = (InternalVersionItem) versionItems.get(id);
@@ -265,7 +266,7 @@
                 return item;
             }
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
     
@@ -350,7 +351,7 @@
      * @param items items updated
      */
     public void itemsUpdated(Collection items) {
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             synchronized (versionItems) {
                 Iterator iter = items.iterator();
@@ -371,7 +372,7 @@
                 }
             }
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
 
@@ -390,11 +391,11 @@
      */
     protected void itemDiscarded(InternalVersionItem item) {
         // evict removed item from cache
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             versionItems.remove(item.getId());
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
 
@@ -473,11 +474,11 @@
      */
     public void stateDestroyed(ItemState destroyed) {
         // evict removed item from cache
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             versionItems.remove(destroyed.getId());
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/XAVersionManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/XAVersionManager.java?rev=750011&r1=750010&r2=750011&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/XAVersionManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/XAVersionManager.java
Wed Mar  4 14:08:48 2009
@@ -46,6 +46,8 @@
 import org.apache.jackrabbit.core.state.NodeReferencesId;
 import org.apache.jackrabbit.core.state.NodeState;
 import org.apache.jackrabbit.core.state.XAItemStateManager;
+import org.apache.jackrabbit.core.state.ISMLocking.ReadLock;
+import org.apache.jackrabbit.core.state.ISMLocking.WriteLock;
 import org.apache.jackrabbit.core.virtual.VirtualItemStateProvider;
 import org.apache.jackrabbit.core.virtual.VirtualNodeState;
 import org.apache.jackrabbit.core.virtual.VirtualPropertyState;
@@ -91,6 +93,11 @@
     private boolean vmgrLocked = false;
 
     /**
+     * The global write lock on the version manager.
+     */
+    private WriteLock vmgrLock;
+
+    /**
      * Creates a new instance of this class.
      */
     public XAVersionManager(VersionManagerImpl vMgr, NodeTypeRegistry ntReg,
@@ -515,8 +522,7 @@
 
     /**
      * Returns an {@link InternalXAResource} that acquires a write lock on the
-     * version manager in {@link InternalXAResource#prepare(TransactionContext)}
-     * if there are any version related items involved in this transaction.
+     * version manager in {@link InternalXAResource#prepare(TransactionContext)}.
      *
      * @return an internal XA resource.
      */
@@ -529,11 +535,8 @@
             }
 
             public void prepare(TransactionContext tx) {
-                Map vItems = (Map) tx.getAttribute(ITEMS_ATTRIBUTE_NAME);
-                if (!vItems.isEmpty()) {
-                    vMgr.acquireWriteLock();
-                    vmgrLocked = true;
-                }
+                vmgrLock = vMgr.acquireWriteLock();
+                vmgrLocked = true;
             }
 
             public void commit(TransactionContext tx) {
@@ -578,7 +581,7 @@
 
             private void internalReleaseWriteLock() {
                 if (vmgrLocked) {
-                    vMgr.releaseWriteLock();
+                    vmgrLock.release();
                     vmgrLocked = false;
                 }
             }
@@ -602,7 +605,7 @@
      */
     private InternalVersionHistoryImpl makeLocalCopy(InternalVersionHistoryImpl history)
             throws RepositoryException {
-        acquireReadLock();
+        ReadLock lock = acquireReadLock();
         try {
             NodeState state = (NodeState) stateMgr.getItemState(history.getId());
             NodeStateEx stateEx = new NodeStateEx(stateMgr, ntReg, state, null);
@@ -610,7 +613,7 @@
         } catch (ItemStateException e) {
             throw new RepositoryException("Unable to make local copy", e);
         } finally {
-            releaseReadLock();
+            lock.release();
         }
     }
 



Mime
View raw message