jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From thom...@apache.org
Subject svn commit: r999298 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state: DefaultISMLocking.java ISMLocking.java SharedItemStateManager.java
Date Tue, 21 Sep 2010 10:12:03 GMT
Author: thomasm
Date: Tue Sep 21 10:12:02 2010
New Revision: 999298

URL: http://svn.apache.org/viewvc?rev=999298&view=rev
Log:
JCR-2753 Deadlock in DefaultISMLocking - reverting revision 995411 and 995412

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/state/ISMLocking.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.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=999298&r1=999297&r2=999298&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
Tue Sep 21 10:12:02 2010
@@ -23,157 +23,203 @@ import javax.transaction.xa.Xid;
 import org.apache.jackrabbit.core.TransactionContext;
 import org.apache.jackrabbit.core.id.ItemId;
 
+import EDU.oswego.cs.dl.util.concurrent.ReadWriteLock;
+import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock;
+import EDU.oswego.cs.dl.util.concurrent.Sync;
+
 /**
- * Default item state locking strategy. The default strategy is simply to use
- * a single coarse-grained read-write lock over the entire workspace.
+ * <code>DefaultISMLocking</code> implements the default locking strategy using
+ * coarse grained locking on an ItemStateManager wide read-write lock. E.g.
+ * while a write lock is held, no read lock can be acquired.
  */
 public class DefaultISMLocking implements ISMLocking {
 
     /**
-     * The read lock instance used by readers to release the acquired lock.
+     * The internal read-write lock.
+	 * Thread concerning ReentrantWriterPreferenceReadWriteLock
      */
-    private final ReadLock readLock = new ReadLock() {
-        public void release() {
-            releaseReadLock();
-        }
-    };
-
+    private final ReadWriteLock rwLock = new ReentrantWriterPreferenceReadWriteLock();
+    
     /**
-     * The write lock instance used by writers to release or downgrade the
-     * acquired lock.
+     * The internal Xid aware read-write lock.
      */
-    private final WriteLock writeLock = new WriteLock() {
-        public void release() {
-            releaseWriteLock(false);
-        }
-        public ReadLock downgrade() {
-            releaseWriteLock(true);
-            return readLock;
-        }
-    };
-
+    private final ReadWriteLock xidRwLock = new XidRWLock();
+    
     /**
-     * Number of writer threads waiting. While greater than zero, no new
-     * readers are allowed to proceed.
+     * {@inheritDoc}
      */
-    private int writersWaiting = 0;
+    public ReadLock acquireReadLock(ItemId id) throws InterruptedException {
+    	if (TransactionContext.getCurrentXid() == null) {
+            return new ReadLockImpl(rwLock.readLock());
+    	} else {
+            return new ReadLockImpl(xidRwLock.readLock());
+    	}
+    }
 
     /**
-     * The thread identifier of the current writer, or <code>null</code> if
-     * no write is in progress. A thread with the same identifier (i.e. the
-     * same thread or another thread in the same transaction) can re-acquire
-     * read or write locks without limitation, while all other readers and
-     * writers remain blocked.
+     * {@inheritDoc}
      */
-    private Object writerId = null;
+    public WriteLock acquireWriteLock(ChangeLog changeLog) throws InterruptedException {
+    	if (TransactionContext.getCurrentXid() == null) {
+    		return new WriteLockImpl(rwLock);
+    	} else {
+    		return new WriteLockImpl(xidRwLock);
+    	}
+    }
 
-    /**
-     * Number of acquired write locks. All the concurrent write locks are
-     * guaranteed to share the same thread identifier (see {@link #writerId}).
-     */
-    private int writerCount = 0;
+    private static final class WriteLockImpl implements WriteLock {
+    	
+    	private ReadWriteLock readWriteLock;
+    	
+    	private WriteLockImpl(ReadWriteLock readWriteLock) throws InterruptedException {
+    		this.readWriteLock = readWriteLock;
+    		this.readWriteLock.writeLock().acquire();
+		}
 
-    /**
-     * Number of acquired read locks.
-     */
-    private int readerCount = 0;
+		/**
+		 * {@inheritDoc}
+		 */
+		public void release() {
+		    this.readWriteLock.writeLock().release();
+		}
 
-    /**
-     * Increments the reader count and returns the acquired read lock once
-     * there are no more writers or the current writer shares the thread id
-     * with this reader.
-     */
-    public synchronized ReadLock acquireReadLock(ItemId id)
-            throws InterruptedException {
-        while (writerId != null
-                ? !isSameId(writerId, getCurrentId()) : writersWaiting > 0) {
-            wait();
-        }
+		/**
+		 * {@inheritDoc}
+		 */
+		public ReadLock downgrade() throws InterruptedException {
+		    ReadLock rLock = new ReadLockImpl(this.readWriteLock.readLock());
+		    release();
+		    return rLock;
+		}
+	}
 
-        readerCount++;
-        return readLock;
-    }
+	private static final class ReadLockImpl implements ReadLock {
 
-    /**
-     * Decrements the reader count and notifies all pending threads if the
-     * lock is now available. Used by the {@link #readLock} instance.
-     */
-    private synchronized void releaseReadLock() {
-        readerCount--;
-        if (readerCount == 0 && writerId == null) {
-            notifyAll();
-        }
-    }
+        private final Sync readLock;
 
-    /**
-     * Increments the writer count, sets the writer identifier and returns
-     * the acquired read lock once there are no other active readers or
-     * writers or the current writer shares the thread id with this writer.
-     */
-    public synchronized WriteLock acquireWriteLock(ChangeLog changeLog)
-            throws InterruptedException {
-        Object currentId = getCurrentId();
-
-        writersWaiting++;
-        try {
-            while (writerId != null
-                    ? !isSameId(writerId, currentId) : readerCount > 0) {
-                wait();
-            }
-        } finally {
-            writersWaiting--;
+        private ReadLockImpl(Sync readLock) throws InterruptedException {
+            this.readLock = readLock;
+            this.readLock.acquire();
         }
 
-        if (writerCount++ == 0) {
-            writerId = currentId;
+        /**
+         * {@inheritDoc}
+         */
+        public void release() {
+            readLock.release();
         }
-        return writeLock;
     }
 
-    /**
-     * Decrements the writer count (and possibly clears the writer identifier)
-     * and notifies all pending threads if the lock is now available. If the
-     * downgrade argument is true, then the reader count is incremented before
-     * notifying any pending threads. Used by the {@link #writeLock} instance.
-     */
-    private synchronized void releaseWriteLock(boolean downgrade) {
-        writerCount--;
-        if (downgrade) {
-            readerCount++;
-        }
-        if (writerCount == 0) {
-            writerId = null;
-            notifyAll();
+	/**
+	 * 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;
+            }
         }
-    }
 
-    /**
-     * Returns the current thread identifier. The identifier is either the
-     * current thread instance or the global transaction identifier when
-     * running under a transaction.
-     *
-     * @return current thread identifier
-     */
-    private Object getCurrentId() {
-        Xid xid = TransactionContext.getCurrentXid();
-        if (xid != null) {
-            return xid.getGlobalTransactionId();
-        } else {
-            return Thread.currentThread();
+        /**
+         * {@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;
+                }
+            }
         }
-    }
 
-    /**
-     * Compares the given thread identifiers for equality.
-     */
-    private boolean isSameId(Object a, Object b) {
-        if (a == b) {
-            return true;
-        } else if (a instanceof byte[] && b instanceof byte[]) {
-            return Arrays.equals((byte[]) a, (byte[]) b);
-        } else {
-            return false;
-        }
+        /**
+         * {@inheritDoc}
+         */
+		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}
+         */
+		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;
+		    	}
+		    }
+		}
     }
-
 }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java?rev=999298&r1=999297&r2=999298&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java
Tue Sep 21 10:12:02 2010
@@ -81,8 +81,10 @@ public interface ISMLocking {
          * used to further release the read lock.
          *
          * @return the read lock downgraded from this write lock.
+         * @throws InterruptedException if the current thread is interrupted
+         *                              while downgrading the write lock.
          */
-        ReadLock downgrade();
+        ReadLock downgrade() throws InterruptedException;
 
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=999298&r1=999297&r2=999298&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
Tue Sep 21 10:12:02 2010
@@ -787,6 +787,9 @@ public class SharedItemStateManager
                     String path = events.getSession().getUserID() + "@" + events.getCommonPath();
                     eventChannel.updateCommitted(this, path);
                 }
+
+            } catch (InterruptedException e) {
+                throw new ItemStateException("Interrupted while downgrading to read lock");
             } finally {
                 if (writeLock != null) {
                     // exception occurred before downgrading lock
@@ -1509,6 +1512,9 @@ public class SharedItemStateManager
                 holdingWriteLock = false;
                 events.dispatch();
             }
+        } catch (InterruptedException e) {
+            String msg = "Unable to downgrade to read lock.";
+            log.error(msg);
         } finally {
             if (holdingWriteLock) {
                 if (wLock != null) {



Mime
View raw message