Index: src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java (revision 0) +++ src/main/java/org/apache/jackrabbit/core/state/DefaultISMLocking.java (revision 0) @@ -0,0 +1,113 @@ +/* + * 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.state; + +import org.apache.jackrabbit.core.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; + +/** + * DefaultISMLocking 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 { + + /** + * JCR-447: deadlock might occur when this manager is still write-locked and events are dispatched. + */ + private boolean noLockHack = false; + + /** + * The internal read-write lock. + */ + 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() + || noLockHack; + } + }; + + /** + * enables or disables the write-lock hack. + * + * @param noLockHack + */ + public void setNoLockHack(boolean noLockHack) { + this.noLockHack = noLockHack; + } + + /** + * {@inheritDoc} + */ + public ReadLock acquireReadLock(ItemId id) + throws InterruptedException { + return new ReadLockImpl(rwLock.readLock()); + } + + /** + * {@inheritDoc} + */ + public WriteLock acquireWriteLock(ChangeLog changeLog) + throws InterruptedException { + return new WriteLock() { + + { + rwLock.writeLock().acquire(); + } + + /** + * {@inheritDoc} + */ + public void release() { + rwLock.writeLock().release(); + } + + /** + * {@inheritDoc} + */ + public ReadLock downgrade() throws InterruptedException { + ReadLock rLock = new ReadLockImpl(rwLock.readLock()); + release(); + return rLock; + } + }; + } + + private static final class ReadLockImpl implements ReadLock { + + private final Sync readLock; + + private ReadLockImpl(Sync readLock) throws InterruptedException { + this.readLock = readLock; + this.readLock.acquire(); + } + + /** + * {@inheritDoc} + */ + public void release() { + readLock.release(); + } + } +} Property changes on: src\main\java\org\apache\jackrabbit\core\state\DefaultISMLocking.java ___________________________________________________________________ Name: svn:eol-style + native Index: src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java (revision 0) +++ src/main/java/org/apache/jackrabbit/core/state/ISMLocking.java (revision 0) @@ -0,0 +1,90 @@ +/* + * 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.state; + +import org.apache.jackrabbit.core.ItemId; + +/** + * ISMLocking defines an interface for a locking strategy of an + * {@link ItemStateManager}. + *

+ * An implementation of ISMLocking must meet the following + * requirements: + *

+ */ +public interface ISMLocking { + + /** + * Acquire a read lock for the given item id. + * @param id an item id. + */ + public ReadLock acquireReadLock(ItemId id) throws InterruptedException; + + /** + * Acquires a write lock for the given changeLog. + * + * @param changeLog the change log + * @return the write lock for the given changeLog. + * @throws InterruptedException if the thread is interrupted while creating + * the write lock. + */ + public WriteLock acquireWriteLock(ChangeLog changeLog) + throws InterruptedException; + + + public interface ReadLock { + + /** + * Releases this lock. + */ + public void release(); + } + + public interface WriteLock { + + /** + * Releases this lock. + */ + public void release(); + + /** + * Downgrades this lock into a read lock. When this method returns this + * write lock is effectively released and the returned read lock must be + * 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. + */ + public ReadLock downgrade() throws InterruptedException; + } +} Property changes on: src\main\java\org\apache\jackrabbit\core\state\ISMLocking.java ___________________________________________________________________ Name: svn:eol-style + native Index: src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java =================================================================== --- src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java (revision 533152) +++ src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java (working copy) @@ -16,8 +16,6 @@ */ package org.apache.jackrabbit.core.state; -import EDU.oswego.cs.dl.util.concurrent.ReadWriteLock; -import EDU.oswego.cs.dl.util.concurrent.ReentrantWriterPreferenceReadWriteLock; import org.apache.jackrabbit.core.ItemId; import org.apache.jackrabbit.core.NodeId; import org.apache.jackrabbit.core.PropertyId; @@ -156,32 +154,14 @@ new VirtualItemStateProvider[0]; /** - * JCR-447: deadlock might occur when this manager is still write-locked and events are dispatched. - */ - private boolean noLockHack = false; - - /** * State change dispatcher. */ private final transient StateChangeDispatcher dispatcher = new StateChangeDispatcher(); /** - * Read-/Write-Lock to synchronize access on this item state manager. + * The locking strategy. */ - private final ReadWriteLock rwLock = - new ReentrantWriterPreferenceReadWriteLock() { - /** - * Allow reader when there is no active writer, or current - * thread owns the write lock (reentrant). - *

- * the 'noLockHack' is only temporary (hopefully) - */ - protected boolean allowReader() { - return activeWriter_ == null - || activeWriter_ == Thread.currentThread() - || noLockHack; - } - }; + private ISMLocking ismLocking; /** * Update event channel. @@ -206,6 +186,8 @@ this.ntReg = ntReg; this.usesReferences = usesReferences; this.rootNodeId = rootNodeId; + //this.ismLocking = new DefaultISMLocking(); + this.ismLocking = new FineGrainedISMLocking(); // create root node state if it doesn't yet exist if (!hasNonVirtualItemState(rootNodeId)) { createRootNodeState(rootNodeId, ntReg); @@ -219,7 +201,9 @@ * @param noLockHack */ public void setNoLockHack(boolean noLockHack) { - this.noLockHack = noLockHack; + if (ismLocking instanceof DefaultISMLocking) { + ((DefaultISMLocking) ismLocking).setNoLockHack(noLockHack); + } } /** @@ -231,6 +215,18 @@ this.eventChannel = eventChannel; } + /** + * Sets a new locking strategy. + * + * @param ismLocking the locking strategy for this item state manager. + */ + public void setISMLocking(ISMLocking ismLocking) { + if (ismLocking == null) { + throw new NullPointerException(); + } + this.ismLocking = ismLocking; + } + //-----------------------------------------------------< ItemStateManager > /** * {@inheritDoc} @@ -238,7 +234,7 @@ public ItemState getItemState(ItemId id) throws NoSuchItemStateException, ItemStateException { - acquireReadLock(); + ISMLocking.ReadLock readLock = acquireReadLock(id); try { // check the virtual root ids (needed for overlay) @@ -258,7 +254,7 @@ } } } finally { - rwLock.readLock().release(); + readLock.release(); } throw new NoSuchItemStateException(id.toString()); } @@ -268,8 +264,9 @@ */ public boolean hasItemState(ItemId id) { + ISMLocking.ReadLock readLock; try { - acquireReadLock(); + readLock = acquireReadLock(id); } catch (ItemStateException e) { return false; } @@ -296,7 +293,7 @@ } } } finally { - rwLock.readLock().release(); + readLock.release(); } return false; } @@ -307,7 +304,7 @@ public NodeReferences getNodeReferences(NodeReferencesId id) throws NoSuchItemStateException, ItemStateException { - acquireReadLock(); + ISMLocking.ReadLock readLock = acquireReadLock(id.getTargetId()); try { // check persistence manager @@ -325,7 +322,7 @@ } } } finally { - rwLock.readLock().release(); + readLock.release(); } // throw @@ -337,8 +334,9 @@ */ public boolean hasNodeReferences(NodeReferencesId id) { + ISMLocking.ReadLock readLock; try { - acquireReadLock(); + readLock = acquireReadLock(id.getTargetId()); } catch (ItemStateException e) { return false; } @@ -359,7 +357,7 @@ } } } finally { - rwLock.readLock().release(); + readLock.release(); } return false; } @@ -494,9 +492,10 @@ private EventStateCollection events; /** - * Flag indicating whether we are holding write lock. + * The write lock we currently hold or null if none is + * hold. */ - private boolean holdingWriteLock; + private ISMLocking.WriteLock writeLock; /** * Map of attributes stored for this update operation. @@ -531,10 +530,9 @@ } try { - acquireWriteLock(); - holdingWriteLock = true; + writeLock = acquireWriteLock(local); } finally { - if (!holdingWriteLock && eventChannel != null) { + if (writeLock == null && eventChannel != null) { eventChannel.updateCancelled(this); } } @@ -697,14 +695,14 @@ } } + ISMLocking.ReadLock readLock = null; try { /* Let the shared item listeners know about the change */ shared.persisted(); // downgrade to read lock - acquireReadLock(); - rwLock.writeLock().release(); - holdingWriteLock = false; + readLock = writeLock.downgrade(); + writeLock = null; /* notify virtual providers about node references */ for (int i = 0; i < virtualNodeReferences.length; i++) { @@ -725,13 +723,15 @@ eventChannel.updateCommitted(this); } + } catch (InterruptedException e) { + throw new ItemStateException("Interrupted while downgrading to read lock"); } finally { - if (holdingWriteLock) { + if (writeLock != null) { // exception occured before downgrading lock - rwLock.writeLock().release(); - holdingWriteLock = false; - } else { - rwLock.readLock().release(); + writeLock.release(); + writeLock = null; + } else if (readLock != null) { + readLock.release(); } } } @@ -770,9 +770,9 @@ state.discard(); } } finally { - if (holdingWriteLock) { - rwLock.writeLock().release(); - holdingWriteLock = false; + if (writeLock != null) { + writeLock.release(); + writeLock = null; } } } @@ -864,8 +864,9 @@ public void externalUpdate(ChangeLog external, EventStateCollection events) { boolean holdingWriteLock = false; + ISMLocking.WriteLock wLock = null; try { - acquireWriteLock(); + wLock = acquireWriteLock(external); holdingWriteLock = true; doExternalUpdate(external); @@ -874,21 +875,25 @@ log.error(msg); } + ISMLocking.ReadLock rLock = null; try { - acquireReadLock(); - rwLock.writeLock().release(); - holdingWriteLock = false; - - events.dispatch(); - } catch (ItemStateException e) { + if (wLock != null) { + rLock = wLock.downgrade(); + holdingWriteLock = false; + events.dispatch(); + } + } catch (InterruptedException e) { String msg = "Unable to downgrade to read lock."; log.error(msg); } finally { if (holdingWriteLock) { - rwLock.writeLock().release(); - holdingWriteLock = false; + if (wLock != null) { + wLock.release(); + } } else { - rwLock.readLock().release(); + if (rLock != null) { + rLock.release(); + } } } @@ -1424,11 +1429,12 @@ /** * Acquires the read lock on this item state manager. * + * @param id the id of the item for which to acquire a read lock. * @throws ItemStateException if the read lock cannot be acquired. */ - private void acquireReadLock() throws ItemStateException { + private ISMLocking.ReadLock acquireReadLock(ItemId id) throws ItemStateException { try { - rwLock.readLock().acquire(); + return ismLocking.acquireReadLock(id); } catch (InterruptedException e) { throw new ItemStateException("Interrupted while acquiring read lock"); } @@ -1437,11 +1443,12 @@ /** * Acquires the write lock on this item state manager. * + * @param changeLog the change log for which to acquire a write lock. * @throws ItemStateException if the write lock cannot be acquired. */ - private void acquireWriteLock() throws ItemStateException { + private ISMLocking.WriteLock acquireWriteLock(ChangeLog changeLog) throws ItemStateException { try { - rwLock.writeLock().acquire(); + return ismLocking.acquireWriteLock(changeLog); } catch (InterruptedException e) { throw new ItemStateException("Interrupted while acquiring write lock"); }