### Eclipse Workspace Patch 1.0 #P jackrabbit-core Index: src/main/java/org/apache/jackrabbit/core/TransactionContext.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/TransactionContext.java (revision 814512) +++ src/main/java/org/apache/jackrabbit/core/TransactionContext.java (working copy) @@ -23,7 +23,6 @@ import javax.transaction.xa.XAException; import javax.transaction.xa.Xid; -import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -339,17 +338,4 @@ public static Xid getCurrentXid() { return (Xid) CURRENT_XID.get(); } - - /** - * Helper Method to check if the given {@link Xid} has the same globalTransactionId - * as the current {@link Xid} bind to the {@link #CURRENT_XID} ThreadLocal - * @param xid Xid to check - * @param fallback if either the given {@link Xid} or the current {@link Xid} is null we can not check if they - * are same, the fallback value will be returned - * @return true if the same otherwise false - */ - public static boolean isCurrentXid(Xid xid, boolean fallback) { - Xid currentXid = (Xid) CURRENT_XID.get(); - return fallback ? true : (currentXid == null || xid == null) ? fallback : Arrays.equals(xid.getGlobalTransactionId(), currentXid.getGlobalTransactionId()); - } } Index: src/main/java/org/apache/jackrabbit/core/lock/LockManagerImpl.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/lock/LockManagerImpl.java (revision 899561) +++ src/main/java/org/apache/jackrabbit/core/lock/LockManagerImpl.java (working copy) @@ -26,7 +26,6 @@ import org.apache.jackrabbit.core.PropertyId; import org.apache.jackrabbit.core.SessionImpl; import org.apache.jackrabbit.core.SessionListener; -import org.apache.jackrabbit.core.TransactionContext; import org.apache.jackrabbit.core.WorkspaceImpl; import org.apache.jackrabbit.core.cluster.ClusterOperation; import org.apache.jackrabbit.core.cluster.LockEventChannel; @@ -61,7 +60,6 @@ import javax.jcr.lock.LockException; import javax.jcr.observation.Event; import javax.jcr.observation.EventIterator; -import javax.transaction.xa.Xid; import java.io.BufferedReader; import java.io.BufferedWriter; @@ -93,46 +91,12 @@ */ private final PathMap lockMap = new PathMap(); - private final ReentrantLock lockMapLock = new ReentrantLock(){ - - private Xid activeXid; - - public void acquire() throws InterruptedException { - if (Thread.interrupted()) throw new InterruptedException(); - Thread caller = Thread.currentThread(); - synchronized(this) { - boolean allow = TransactionContext.isCurrentXid(activeXid, caller == owner_); - if (allow) { - ++holds_; - } else { - try { - while (owner_ != null) - wait(); - owner_ = caller; - activeXid = (Xid) TransactionContext.getCurrentXid(); - holds_ = 1; - } catch (InterruptedException ex) { - notify(); - throw ex; - } - } - } - } - - public synchronized void release() { - boolean allow = TransactionContext.isCurrentXid(activeXid, Thread.currentThread() == owner_); - if (!allow) - throw new Error("Illegal Lock usage"); - - if (--holds_ == 0) { - owner_ = null; - activeXid = null; - notify(); - } - } - }; - /** + * Lock to path map. + */ + private final ReentrantLock lockMapLock = new ReentrantLock(); + + /** * System session */ private final SessionImpl sysSession; Index: src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java (revision 899564) +++ src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java (working copy) @@ -22,9 +22,8 @@ import org.apache.jackrabbit.core.ItemId; import org.apache.jackrabbit.core.TransactionContext; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import EDU.oswego.cs.dl.util.concurrent.ReadWriteLock; import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock; import EDU.oswego.cs.dl.util.concurrent.Sync; @@ -36,54 +35,64 @@ public class DefaultISMLocking implements ISMLocking { /** - * Logger instance + * The internal read-write lock. */ - private static final Logger log = LoggerFactory.getLogger(DefaultISMLocking.class); - + private final ReadWriteLock rwLock = new RWLock(); + /** - * The internal read-write lock. + * The internal Xid aware read-write lock. */ - private final RWLock rwLock = new RWLock(); - + private final ReadWriteLock xidRwLock = new XidRWLock(); + /** * {@inheritDoc} */ - public ReadLock acquireReadLock(ItemId id) - throws InterruptedException { - return new ReadLockImpl(rwLock.readLock()); + public ReadLock acquireReadLock(ItemId id) throws InterruptedException { + if (TransactionContext.getCurrentXid() == null) { + return new ReadLockImpl(rwLock.readLock()); + } else { + return new ReadLockImpl(xidRwLock.readLock()); + } } /** * {@inheritDoc} */ - public WriteLock acquireWriteLock(ChangeLog changeLog) - throws InterruptedException { - return new WriteLock() { + public WriteLock acquireWriteLock(ChangeLog changeLog) throws InterruptedException { + if (TransactionContext.getCurrentXid() == null) { + return new WriteLockImpl(rwLock); + } else { + return new WriteLockImpl(xidRwLock); + } + } - { - rwLock.writeLock().acquire(); - rwLock.setActiveXid(TransactionContext.getCurrentXid()); - } + private static final class WriteLockImpl implements WriteLock { + + private ReadWriteLock readWriteLock; + + private WriteLockImpl(ReadWriteLock readWriteLock) throws InterruptedException { + this.readWriteLock = readWriteLock; + this.readWriteLock.writeLock().acquire(); + } - /** - * {@inheritDoc} - */ - public void release() { - rwLock.writeLock().release(); - } + /** + * {@inheritDoc} + */ + public void release() { + this.readWriteLock.writeLock().release(); + } - /** - * {@inheritDoc} - */ - public ReadLock downgrade() throws InterruptedException { - ReadLock rLock = new ReadLockImpl(rwLock.readLock()); - release(); - return rLock; - } - }; - } + /** + * {@inheritDoc} + */ + public ReadLock downgrade() throws InterruptedException { + ReadLock rLock = new ReadLockImpl(this.readWriteLock.readLock()); + release(); + return rLock; + } + } - private static final class ReadLockImpl implements ReadLock { + private static final class ReadLockImpl implements ReadLock { private final Sync readLock; @@ -100,45 +109,75 @@ } } + /** + * Thread concerning ReentrantWriterPreferenceReadWriteLock + */ private static final class RWLock extends 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(); + } + } + + /** + * Xid concerning ReentrantWriterPreferenceReadWriteLock + */ + private static final class XidRWLock extends ReentrantWriterPreferenceReadWriteLock { + private Xid activeXid; /** - * Allow reader when there is no active writer, or current thread owns + * 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() { - return TransactionContext.isCurrentXid(activeXid, (activeWriter_ == null || activeWriter_ == Thread.currentThread())); + Xid currentXid = TransactionContext.getCurrentXid(); + return activeXid == null || isSameGlobalTx(currentXid); } /** - * Sets the active Xid - * @param xid - */ - synchronized void setActiveXid(Xid xid) { - if (activeXid != null && xid != null) { - boolean sameGTI = Arrays.equals(activeXid.getGlobalTransactionId(), xid.getGlobalTransactionId()); - if (!sameGTI) { - log.warn("Unable to set the ActiveXid while a other one is associated with a different GloalTransactionId with this RWLock."); - return; - } + * {@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; } - activeXid = xid; } /** * {@inheritDoc} - * - * If there are no more writeHolds the activeXid will be set to null */ protected synchronized Signaller endWrite() { --writeHolds_; if (writeHolds_ > 0) { // still being held - return null; + return null; } else { - activeXid = null; - activeWriter_ = null; + activeXid = null; if (waitingReaders_ > 0 && allowReader()) { return readerLock_; } else if (waitingWriters_ > 0) { @@ -148,5 +187,52 @@ } } } + + /** + * {@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; + } + } + } } }