Author: mircea.markus
Date: 2008-02-26 07:39:28 -0500 (Tue, 26 Feb 2008)
New Revision: 5371
Modified:
core/trunk/src/main/java/org/jboss/cache/lock/IdentityLock.java
core/trunk/src/main/java/org/jboss/cache/lock/LockMap.java
core/trunk/src/test/java/org/jboss/cache/lock/IdentityLockTest.java
Log:
Fixed an concurrency bug introduced in 2.1.4.CR4.
The issues is caused by the fact that the list of readLockOwners returned by LockMap is
not guarded against modifications. This might cause ConcurrentModificationExceptions being
raised at different points in code (basically on any usage of
IdentityLock.getReadOwners)
Modified: core/trunk/src/main/java/org/jboss/cache/lock/IdentityLock.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/IdentityLock.java 2008-02-21 04:28:29
UTC (rev 5370)
+++ core/trunk/src/main/java/org/jboss/cache/lock/IdentityLock.java 2008-02-26 12:39:28
UTC (rev 5371)
@@ -145,6 +145,11 @@
return map_.writerOwner();
}
+ public LockMap getLockMap()
+ {
+ return map_;
+ }
+
/**
* Acquire a write lock with a timeout of <code>timeout</code>
milliseconds.
* Note that if the current owner owns a read lock, it will be upgraded
@@ -456,10 +461,6 @@
sb.append("read owners=").append(read_owners);
printed_read_owners = true;
}
- else
- {
- read_owners = null;
- }
Object write_owner = lock_ != null ? getWriterOwner() : null;
if (write_owner != null)
Modified: core/trunk/src/main/java/org/jboss/cache/lock/LockMap.java
===================================================================
--- core/trunk/src/main/java/org/jboss/cache/lock/LockMap.java 2008-02-21 04:28:29 UTC
(rev 5370)
+++ core/trunk/src/main/java/org/jboss/cache/lock/LockMap.java 2008-02-26 12:39:28 UTC
(rev 5371)
@@ -6,10 +6,7 @@
*/
package org.jboss.cache.lock;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
/**
* Provide lock ownership mapping.
@@ -116,8 +113,16 @@
*/
public Collection<Object> readerOwners()
{
- return readOwnerList_;
-// return Collections.unmodifiableSet(readOwnerList_);
+ //make a defense copy, otherwise ConcurrentModificationException might appear while
client code is iterating
+ // the original map (see
IdentityLockTest.testConcurrentModificationOfReadLocksAndToString)
+ List readOwnersListCopy;
+ synchronized (readOwnerList_) //readOwnerList_ is a sync list, make sure noone
modifies it while we make a copy of it.
+ {
+ readOwnersListCopy = new ArrayList(readOwnerList_);
+ readOwnersListCopy.addAll(readOwnerList_);
+ }
+ return Collections.unmodifiableList(readOwnersListCopy);
+// return readOwnerList_;
}
public void releaseReaderOwners(LockStrategy lock)
Modified: core/trunk/src/test/java/org/jboss/cache/lock/IdentityLockTest.java
===================================================================
--- core/trunk/src/test/java/org/jboss/cache/lock/IdentityLockTest.java 2008-02-21
04:28:29 UTC (rev 5370)
+++ core/trunk/src/test/java/org/jboss/cache/lock/IdentityLockTest.java 2008-02-26
12:39:28 UTC (rev 5371)
@@ -532,4 +532,66 @@
logger_.info("-- [" + Thread.currentThread() + "]: " + msg);
}
+ /**
+ * When IdentityLock.toString is called and readLockOwners are modified an
ConcurrentModificationException exception
+ * might be thrown.
+ */
+ public void testConcurrentModificationOfReadLocksAndToString() throws Exception
+ {
+ final IdentityLock iLock = (IdentityLock) lock_;
+ final LockMap lockMap = iLock.getLockMap();
+ final int opCount = 100000;
+ final Thread readLockChanger = new Thread()
+ {
+ public void run()
+ {
+ for (int i = 0; i < opCount; i++)
+ {
+ if (i%10 ==0) System.out.println("readLockChanger loop# " + i);
+ lockMap.addReader(new Object());
+ }
+ }
+ };
+
+ final boolean[] flags = new boolean[]{false, false}; //{testFailure,
stopTheThread}
+ Thread toStringProcessor = new Thread()
+ {
+ public void run()
+ {
+ long initialTime = System.currentTimeMillis();
+ for (int i = 0; i < opCount; i++)
+ {
+ if (i%10 ==0)
+ {
+ System.out.println("toStringProcessor loop# " + i + ",
" + (System.currentTimeMillis() - initialTime));
+ initialTime = System.currentTimeMillis();
+ }
+ try
+ {
+ iLock.toString(new StringBuffer(), false);
+ } catch (Exception e)
+ {
+ e.printStackTrace();
+ flags[0] = true;
+ break;
+ }
+ if (flags[1]) break;
+ }
+ }
+ };
+ toStringProcessor.start();
+ System.out.println("toStringProcessor started");
+ readLockChanger.start();
+ System.out.println("readLockChanger started");
+
+ readLockChanger.join();
+ flags[1] = true;//stopping the toStringProcessor
+ System.out.println("readLockChanger stopped");
+
+ toStringProcessor.join();
+ System.out.println("toStringProcessor stopped");
+
+ System.out.println("flags[0]=" + flags[0]);
+ assertFalse(flags[0]);
+ }
}
Show replies by date