[jboss-cvs] JBossCache/src/org/jboss/cache/lock ...
Manik Surtani
msurtani at belmont.prod.atl2.jboss.com
Wed Aug 30 15:09:56 EDT 2006
User: msurtani
Date: 06/08/30 15:09:56
Modified: src/org/jboss/cache/lock IdentityLock.java LockUtil.java
Log:
removed unnenessary (IDE-generated) TODOs that were becoming noisy
refactored identity lock constructors
Revision Changes Path
1.14 +99 -86 JBossCache/src/org/jboss/cache/lock/IdentityLock.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: IdentityLock.java
===================================================================
RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/lock/IdentityLock.java,v
retrieving revision 1.13
retrieving revision 1.14
diff -u -b -r1.13 -r1.14
--- IdentityLock.java 19 Jul 2006 21:34:44 -0000 1.13
+++ IdentityLock.java 30 Aug 2006 19:09:56 -0000 1.14
@@ -10,9 +10,11 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jboss.cache.Fqn;
-import org.jboss.cache.TreeCache;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
/**
* Lock object which grants and releases locks, and associates locks with
@@ -22,7 +24,7 @@
* wants to acquire the lock, it is immediately granted. When an owner
* different from the one currently owning the lock wants to release the lock,
* we do nothing (no-op).
- * <p>
+ * <p/>
* Consider the following example:
* <pre>
* FIFOSemaphore lock=new FIFOSemaphore(1);
@@ -53,7 +55,7 @@
*
* @author <a href="mailto:bela at jboss.org">Bela Ban</a> Apr 11, 2003
* @author Ben Wang July 2003
- * @version $Revision: 1.13 $
+ * @version $Revision: 1.14 $
*/
public class IdentityLock
{
@@ -65,54 +67,40 @@
private final boolean mustReacquireRead_;
private Fqn fqn_;
+
/**
- * Constructs a new lock.
- * @param cache cache for deciding lock strategy
- * @param fqn ignored.
+ * Creates a new Identity lock based on the lock level set in the {@link LockStrategyFactory}
*/
- public IdentityLock(TreeCache cache, Fqn fqn)
+ public IdentityLock(Fqn fqn)
{
- this(cache);
- this.fqn_ = fqn;
+ this(LockStrategyFactory.getLockStrategy(), fqn);
+ log.trace("Using default lock level");
}
/**
- * Constructs a new lock.
- * @param cache cache for deciding lock strategy
+ * Creates a new IdentityLock based on the lock level in force.
*
- * TODO remove this constructor
+ * @param level
*/
- public IdentityLock(TreeCache cache)
+ public IdentityLock(IsolationLevel level, Fqn fqn)
{
- if(cache == null) {
- if(trace) {
- log.trace("Cache instance is null. Use default lock strategy");
- }
- lock_ = LockStrategyFactory.getLockStrategy();
- // TODO This is ugly; builds in the assumption that default strategy
- // is REPEATABLE_READ
- mustReacquireRead_ = false;
- } else {
- IsolationLevel level = cache.getConfiguration().getIsolationLevel();
- lock_ = LockStrategyFactory.getLockStrategy(level);
- mustReacquireRead_ = (level == IsolationLevel.READ_COMMITTED);
- }
- map_ = new LockMap();
- }
+ this(LockStrategyFactory.getLockStrategy(level), fqn);
+ }
- /** For testing only */
- public IdentityLock(IsolationLevel level) {
- lock_ = LockStrategyFactory.getLockStrategy(level);
- mustReacquireRead_ = (level == IsolationLevel.READ_COMMITTED);
+ private IdentityLock(LockStrategy strategy, Fqn fqn)
+ {
+ lock_ = strategy;
+ mustReacquireRead_ = strategy instanceof LockStrategyReadCommitted;
map_ = new LockMap();
- fqn_ = Fqn.ROOT;
+ fqn_ = fqn == null ? Fqn.ROOT : fqn;
}
/**
* Returns the FQN for this lock, may be <code>null</code>.
*/
- public Fqn getFqn() {
+ public Fqn getFqn()
+ {
return fqn_;
}
@@ -145,57 +133,69 @@
*
* @param caller Can't be null.
* @param timeout
+ * @return boolean True if lock was acquired and was not held before, false if lock was held
* @throws LockingException
* @throws TimeoutException
- * @return boolean True if lock was acquired and was not held before, false if lock was held
*/
public boolean acquireWriteLock(Object caller, long timeout) throws LockingException, TimeoutException, InterruptedException
{
- if (caller == null) {
+ if (caller == null)
+ {
throw new IllegalArgumentException("acquireWriteLock(): null caller");
}
- if (map_.isOwner(caller, LockMap.OWNER_WRITE)) {
+ if (map_.isOwner(caller, LockMap.OWNER_WRITE))
+ {
if (trace)
log.trace("acquireWriteLock(): caller already owns lock for " + getFqn() + " (caller=" + caller + ')');
return false; // owner already has the lock
}
// Check first if we need to upgrade
- if (map_.isOwner(caller, LockMap.OWNER_READ)) {
+ if (map_.isOwner(caller, LockMap.OWNER_READ))
+ {
// Currently is a reader owner. Obtain the writer ownership then.
Sync wLock;
- try {
- if(trace)
+ try
+ {
+ if (trace)
log.trace("upgrading RL to WL for " + caller + ", timeout=" + timeout + ", locks: " + map_.printInfo());
wLock = lock_.upgradeLockAttempt(timeout);
- } catch (UpgradeException ue) {
- String errStr="acquireWriteLock(): lock upgrade failed for " + getFqn() + " (caller=" + caller + ')';
+ }
+ catch (UpgradeException ue)
+ {
+ String errStr = "acquireWriteLock(): lock upgrade failed for " + getFqn() + " (caller=" + caller + ')';
log.error(errStr, ue);
throw new UpgradeException(errStr, ue);
}
- if (wLock == null) {
+ if (wLock == null)
+ {
release(caller); // bug fix: remember to release the read lock before throwing the exception
map_.removeReader(caller);
- String errStr="upgrade lock for " + getFqn() + " could not be acquired after " + timeout + " ms." +
+ String errStr = "upgrade lock for " + getFqn() + " could not be acquired after " + timeout + " ms." +
" Lock map ownership " + map_.printInfo() + " (caller=" + caller + ')';
log.error(errStr);
throw new UpgradeException(errStr);
}
- try {
+ try
+ {
if (trace)
log.trace("upgrading lock for " + getFqn());
map_.upgrade(caller);
- } catch (OwnerNotExistedException ex) {
+ }
+ catch (OwnerNotExistedException ex)
+ {
throw new UpgradeException("Can't upgrade lock to WL for " + getFqn() + ", error in LockMap.upgrade(): " + ex);
}
}
- else {
+ else
+ {
// Not a current reader owner. Obtain the writer ownership then.
boolean rc = lock_.writeLock().attempt(timeout);
// we don't need to synchronize from here on because we own the semaphore
- if (rc == false) {
+ if (!rc)
+ {
String errStr = "write lock for " + getFqn() + " could not be acquired after " + timeout + " ms. " +
"Locks: " + map_.printInfo() + " (caller=" + caller + ", lock info: " + toString(true) + ')';
log.error(errStr);
@@ -211,16 +211,17 @@
*
* @param caller Can't be null.
* @param timeout
+ * @return boolean True if lock was acquired and was not held before, false if lock was held
* @throws LockingException
* @throws TimeoutException
- * @return boolean True if lock was acquired and was not held before, false if lock was held
*/
public boolean acquireReadLock(Object caller, long timeout)
throws LockingException, TimeoutException, InterruptedException
{
boolean rc;
- if (caller == null) {
+ if (caller == null)
+ {
throw new IllegalArgumentException("owner is null");
}
@@ -232,13 +233,16 @@
if (!hasRequired)
hasRead = map_.isOwner(caller, LockMap.OWNER_READ);
}
- else if (map_.isOwner(caller, LockMap.OWNER_ANY)) {
+ else if (map_.isOwner(caller, LockMap.OWNER_ANY))
+ {
hasRequired = true;
}
- if (hasRequired) {
- if (trace) {
- StringBuffer sb=new StringBuffer(64);
+ if (hasRequired)
+ {
+ if (trace)
+ {
+ StringBuffer sb = new StringBuffer(64);
sb.append("acquireReadLock(): caller ").append(caller).append(" already owns lock for ").append(getFqn());
log.trace(sb.toString());
}
@@ -248,7 +252,8 @@
rc = lock_.readLock().attempt(timeout);
// we don't need to synchronize from here on because we own the semaphore
- if (rc == false) {
+ if (!rc)
+ {
StringBuffer sb = new StringBuffer();
sb.append("read lock for ").append(getFqn()).append(" could not be acquired by ").append(caller);
sb.append(" after ").append(timeout).append(" ms. " + "Locks: ").append(map_.printInfo());
@@ -270,16 +275,19 @@
*/
public void release(Object caller)
{
- if (caller == null) {
+ if (caller == null)
+ {
throw new IllegalArgumentException("IdentityLock.release(): null owner object.");
}
// Check whether to release reader or writer lock.
- if (map_.isOwner(caller, LockMap.OWNER_READ)) {
+ if (map_.isOwner(caller, LockMap.OWNER_READ))
+ {
map_.removeReader(caller);
lock_.readLock().release();
}
- else if (map_.isOwner(caller, LockMap.OWNER_WRITE)) {
+ else if (map_.isOwner(caller, LockMap.OWNER_WRITE))
+ {
map_.removeWriter();
lock_.writeLock().release();
}
@@ -291,15 +299,18 @@
public void releaseAll()
{
Collection col;
- try {
- if ((map_.writerOwner()) != null) {
+ try
+ {
+ if ((map_.writerOwner()) != null)
+ {
// lock_.readLock().release();
lock_.writeLock().release();
}
map_.releaseReaderOwners(lock_);
}
- finally {
+ finally
+ {
map_.removeAll();
}
}
@@ -353,7 +364,7 @@
public String toString(boolean print_lock_details)
{
- StringBuffer sb=new StringBuffer();
+ StringBuffer sb = new StringBuffer();
toString(sb, print_lock_details);
return sb.toString();
}
@@ -365,35 +376,37 @@
public void toString(StringBuffer sb, boolean print_lock_details)
{
- boolean printed_read_owners=false;
- Collection read_owners=lock_ != null ? getReaderOwners() : null;
- if(read_owners != null && read_owners.size() > 0) {
+ boolean printed_read_owners = false;
+ Collection read_owners = lock_ != null ? getReaderOwners() : null;
+ if (read_owners != null && read_owners.size() > 0)
+ {
// Fix for JBCACHE-310 -- can't just call new ArrayList(read_owners) :(
// Creating the ArrayList and calling addAll doesn't work either
// Looking at the details of how this is implemented vs. the 2 prev
// options, this doesn't look like it should be slower
Iterator iter = read_owners.iterator();
read_owners = new ArrayList(read_owners.size());
- while(iter.hasNext())
+ while (iter.hasNext())
read_owners.add(iter.next());
sb.append("read owners=").append(read_owners);
- printed_read_owners=true;
+ printed_read_owners = true;
}
else
- read_owners=null;
+ read_owners = null;
- Object write_owner=lock_ != null ? getWriterOwner() : null;
- if(write_owner != null) {
- if(printed_read_owners)
+ Object write_owner = lock_ != null ? getWriterOwner() : null;
+ if (write_owner != null)
+ {
+ if (printed_read_owners)
sb.append(", ");
sb.append("write owner=").append(write_owner);
}
- if(read_owners == null && write_owner == null)
+ if (read_owners == null && write_owner == null)
{
sb.append("<unlocked>");
}
- if(print_lock_details)
+ if (print_lock_details)
{
sb.append(" (").append(lock_.toString()).append(')');
}
1.3 +22 -33 JBossCache/src/org/jboss/cache/lock/LockUtil.java
(In the diff below, changes in quantity of whitespace are not shown.)
Index: LockUtil.java
===================================================================
RCS file: /cvsroot/jboss/JBossCache/src/org/jboss/cache/lock/LockUtil.java,v
retrieving revision 1.2
retrieving revision 1.3
diff -u -b -r1.2 -r1.3
--- LockUtil.java 14 Aug 2006 17:52:34 -0000 1.2
+++ LockUtil.java 30 Aug 2006 19:09:56 -0000 1.3
@@ -1,12 +1,5 @@
package org.jboss.cache.lock;
-import java.util.Collection;
-import java.util.Iterator;
-
-import javax.transaction.Status;
-import javax.transaction.Transaction;
-import javax.transaction.TransactionManager;
-
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.jboss.cache.DataNode;
@@ -15,6 +8,12 @@
import org.jboss.cache.TreeCache;
import org.jboss.cache.statetransfer.StateTransferManager;
+import javax.transaction.Status;
+import javax.transaction.Transaction;
+import javax.transaction.TransactionManager;
+import java.util.Collection;
+import java.util.Iterator;
+
public abstract class LockUtil
{
private final static Log log = LogFactory.getLog(StateTransferManager.class);
@@ -24,15 +23,6 @@
public static final int STATUS_BROKEN = Integer.MIN_VALUE;
}
- /**
- *
- *
- * @param lock
- * @param gtx
- * @param cache TODO
- * @param localAddress
- * @return <code>true</code> if a lock was broken, <code>false</code> otherwise
- */
public static boolean breakTransactionLock(IdentityLock lock,
GlobalTransaction gtx,
boolean localTx,
@@ -68,7 +58,6 @@
*
* @param node the node
* @param newOwner the new owner (usually a Thread or GlobalTransaction)
- * @param cache TODO
* @param lockChildren <code>true</code> if this method should be recursively
* applied to <code>node</code>'s children.
*/
More information about the jboss-cvs-commits
mailing list