ozeigermann 2005/01/13 08:44:03
Modified: transaction/src/java/org/apache/commons/transaction/locking
GenericLockManager.java
transaction/src/test/org/apache/commons/transaction/locking
GenericLockTest.java
Log:
Fixed deadlock hazard in deadlock detection caused by interleaving access
to locks set of an owner and added test case that revealed it
Revision Changes Path
1.20 +14 -9
jakarta-commons/transaction/src/java/org/apache/commons/transaction/locking/GenericLockManager.java
Index: GenericLockManager.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/transaction/src/java/org/apache/commons/transaction/locking/GenericLockManager.java,v
retrieving revision 1.19
retrieving revision 1.20
diff -u -r1.19 -r1.20
--- GenericLockManager.java 9 Jan 2005 19:33:53 -0000 1.19
+++ GenericLockManager.java 13 Jan 2005 16:44:03 -0000 1.20
@@ -281,12 +281,17 @@
protected void releaseAllNoTimeOutReset(Object ownerId) {
Set locks = (Set) globalOwners.get(ownerId);
if (locks != null) {
+ Collection locksCopy;
+ // need to copy in order not to interfere with wouldDeadlock
+ // possibly called by
+ // other threads
synchronized (locks) {
- for (Iterator it = locks.iterator(); it.hasNext();) {
- GenericLock lock = (GenericLock) it.next();
- lock.release(ownerId);
- it.remove();
- }
+ locksCopy = new ArrayList(locks);
+ }
+ for (Iterator it = locksCopy.iterator(); it.hasNext();) {
+ GenericLock lock = (GenericLock) it.next();
+ lock.release(ownerId);
+ it.remove();
}
}
}
1.12 +77 -6
jakarta-commons/transaction/src/test/org/apache/commons/transaction/locking/GenericLockTest.java
Index: GenericLockTest.java
===================================================================
RCS file:
/home/cvs/jakarta-commons/transaction/src/test/org/apache/commons/transaction/locking/GenericLockTest.java,v
retrieving revision 1.11
retrieving revision 1.12
diff -u -r1.11 -r1.12
--- GenericLockTest.java 8 Jan 2005 18:56:03 -0000 1.11
+++ GenericLockTest.java 13 Jan 2005 16:44:03 -0000 1.12
@@ -262,6 +262,9 @@
sLogger.logInfo("\n\nChecking detection of indirect deadlock \n\n");
+ final String jamowner1 = "jamowner1";
+ final String jamowner2 = "jamowner2";
+
final String owner1 = "owner1";
final String owner2 = "owner2";
final String owner3 = "owner3";
@@ -274,7 +277,7 @@
final ReadWriteLockManager manager = new
ReadWriteLockManager(sLogger,
TIMEOUT);
- final RendezvousBarrier restart = new RendezvousBarrier("restart",
3, TIMEOUT, sLogger);
+ final RendezvousBarrier restart = new RendezvousBarrier("restart",
5, TIMEOUT, sLogger);
final TurnBarrier cb = new TurnBarrier("cb1", TIMEOUT, sLogger, 1);
@@ -282,6 +285,74 @@
System.out.print(".");
+ // thread that accesses lock of res1 just to cause interference
and
+ // possibly detect concurrency problems
+ Thread jamThread1 = new Thread(new Runnable() {
+ public void run() {
+ try {
+ for (int i = 0; i < 10; i++) {
+ manager.readLock(jamowner1, res1);
+ Thread.sleep(10);
+ manager.releaseAll(jamowner1);
+ Thread.sleep(10);
+ manager.writeLock(jamowner1, res1);
+ Thread.sleep(10);
+ manager.releaseAll(jamowner1);
+ Thread.sleep(10);
+ }
+ } catch (LockException le) {
+ fail("Jam Thread should not fail");
+ } catch (InterruptedException ie) {
+ } finally {
+ manager.releaseAll(jamowner1);
+ synchronized (restart) {
+ try {
+ synchronized (restart) {
+ restart.meet();
+ restart.reset();
+ }
+ } catch (InterruptedException ie) {}
+ }
+ }
+ }
+ }, "Jam Thread #1");
+
+ jamThread1.start();
+
+ // thread that accesses lock of res1 just to cause interference
and
+ // possibly detect concurrency problems
+ Thread jamThread2 = new Thread(new Runnable() {
+ public void run() {
+ try {
+ for (int i = 0; i < 10; i++) {
+ manager.writeLock(jamowner2, res1);
+ Thread.sleep(10);
+ manager.releaseAll(jamowner2);
+ Thread.sleep(10);
+ manager.readLock(jamowner2, res1);
+ Thread.sleep(10);
+ manager.releaseAll(jamowner2);
+ Thread.sleep(10);
+ }
+ } catch (LockException le) {
+ fail("Jam Thread should not fail");
+ } catch (InterruptedException ie) {
+ } finally {
+ manager.releaseAll(jamowner2);
+ synchronized (restart) {
+ try {
+ synchronized (restart) {
+ restart.meet();
+ restart.reset();
+ }
+ } catch (InterruptedException ie) {}
+ }
+ }
+ }
+ }, "Jam Thread #2");
+
+ jamThread2.start();
+
Thread t1 = new Thread(new Runnable() {
public void run() {
try {
@@ -368,7 +439,7 @@
// XXX in special scenarios the current implementation might
cause more than one
// owner to be a deadlock victim
if (deadlockCnt != 1) {
- sLogger.logWarning("More than one thread was deadlock
victim!");
+ sLogger.logWarning("\nMore than one thread was deadlock
victim!\n");
}
assertTrue(deadlockCnt >= 1);
deadlockCnt = 0;
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
