All,
Avi Drissman ([EMAIL PROTECTED]) pointed out a problem with the current
implementation of org.apache.avalon.excalibur.concurrent.ReadWriteLock: If
you hold a read lock, and a thread waiting for a write lock is interrupted,
there is no way to aquire a read lock again.
This patch fixes the problem, and a syntactic error in build.xml.
/LS
cvs server: Diffing .
Index: build.xml
===================================================================
RCS file: /home/cvspublic/jakarta-avalon-excalibur/build.xml,v
retrieving revision 1.70
diff -u -r1.70 build.xml
--- build.xml 2001/10/30 22:03:49 1.70
+++ build.xml 2001/10/31 08:26:09
@@ -169,9 +169,6 @@
</available>
<available property="servlet.present" classname="javax.servlet.Servlet">
<classpath refid="project.class.path"/>
- <available property="jaxen.present" classname="org.jaxen.dom.XPath">
- <classpath refid="project.class.path"/>
- </available>
</available>
<available property="jms.present" classname="javax.jms.Queue">
<classpath refid="project.class.path"/>
cvs server: Diffing lib
cvs server: Diffing src
cvs server: Diffing src/java
cvs server: Diffing src/java/org
cvs server: Diffing src/java/org/apache
cvs server: Diffing src/java/org/apache/avalon
cvs server: Diffing src/java/org/apache/avalon/excalibur
cvs server: Diffing src/java/org/apache/avalon/excalibur/cli
cvs server: Diffing src/java/org/apache/avalon/excalibur/collections
cvs server: Diffing src/java/org/apache/avalon/excalibur/component
cvs server: Diffing src/java/org/apache/avalon/excalibur/concurrent
Index: src/java/org/apache/avalon/excalibur/concurrent/ReadWriteLock.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-excalibur/src/java/org/apache/avalon/excalibur/concurrent/ReadWriteLock.java,v
retrieving revision 1.4
diff -u -r1.4 ReadWriteLock.java
--- src/java/org/apache/avalon/excalibur/concurrent/ReadWriteLock.java
2001/08/10 17:12:52 1.4
+++ src/java/org/apache/avalon/excalibur/concurrent/ReadWriteLock.java
2001/10/31 08:26:10
@@ -79,12 +79,18 @@
synchronized ( m_lock )
{
m_numWaitingForWrite++;
- while ( m_numReadLocksHeld != 0 )
+ try
{
- m_lock.wait();
+ while ( m_numReadLocksHeld != 0 )
+ {
+ m_lock.wait();
+ }
+ m_numReadLocksHeld = -1;
}
- m_numReadLocksHeld = -1;
- m_numWaitingForWrite--;
+ finally
+ {
+ m_numWaitingForWrite--;
+ }
}
}
cvs server: Diffing src/java/org/apache/avalon/excalibur/datasource
cvs server: Diffing src/java/org/apache/avalon/excalibur/i18n
cvs server: Diffing src/java/org/apache/avalon/excalibur/io
cvs server: Diffing src/java/org/apache/avalon/excalibur/logger
cvs server: Diffing src/java/org/apache/avalon/excalibur/logger/factory
cvs server: Diffing src/java/org/apache/avalon/excalibur/monitor
cvs server: Diffing src/java/org/apache/avalon/excalibur/naming
cvs server: Diffing src/java/org/apache/avalon/excalibur/naming/memory
cvs server: Diffing src/java/org/apache/avalon/excalibur/naming/rmi
cvs server: Diffing src/java/org/apache/avalon/excalibur/naming/rmi/server
cvs server: Diffing src/java/org/apache/avalon/excalibur/pool
cvs server: Diffing src/java/org/apache/avalon/excalibur/property
cvs server: Diffing src/java/org/apache/avalon/excalibur/proxy
cvs server: Diffing src/java/org/apache/avalon/excalibur/testcase
cvs server: Diffing src/java/org/apache/avalon/excalibur/xml
cvs server: Diffing src/java/org/apache/avalon/excalibur/xml/xpath
cvs server: Diffing src/scratchpad
cvs server: Diffing src/scratchpad/org
cvs server: Diffing src/scratchpad/org/apache
cvs server: Diffing src/scratchpad/org/apache/avalon
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/cache
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/cache/doc-files
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/cache/test
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/catalog
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/container
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/extension
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/extension/test
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/i18n
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/i18n/test
cvs server: cannot find
src/scratchpad/org/apache/avalon/excalibur/i18n/test/XMLResourceBundleTestCase.java
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/i18n/util
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/lang
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/pipeline
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/thread
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/thread/impl
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/util
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/util/test
cvs server: Diffing src/scratchpad/org/apache/avalon/excalibur/vfs
cvs server: Diffing src/test
cvs server: Diffing src/test/org
cvs server: Diffing src/test/org/apache
cvs server: Diffing src/test/org/apache/avalon
cvs server: Diffing src/test/org/apache/avalon/excalibur
cvs server: Diffing src/test/org/apache/avalon/excalibur/cli
cvs server: Diffing src/test/org/apache/avalon/excalibur/cli/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/collections
cvs server: Diffing src/test/org/apache/avalon/excalibur/collections/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/concurrent
cvs server: Diffing src/test/org/apache/avalon/excalibur/concurrent/test
Index:
src/test/org/apache/avalon/excalibur/concurrent/test/ReadWriteLockTestCase.java
===================================================================
RCS file:
/home/cvspublic/jakarta-avalon-excalibur/src/test/org/apache/avalon/excalibur/concurrent/test/ReadWriteLockTestCase.java,v
retrieving revision 1.4
diff -u -r1.4 ReadWriteLockTestCase.java
---
src/test/org/apache/avalon/excalibur/concurrent/test/ReadWriteLockTestCase.java
2001/10/25 16:00:48 1.4
+++
src/test/org/apache/avalon/excalibur/concurrent/test/ReadWriteLockTestCase.java
2001/10/31 08:26:15
@@ -28,17 +28,17 @@
{
protected ReadWriteLock m_lock;
protected boolean m_success = false;
-
+
public TriesWriteLock(ReadWriteLock lock)
{
m_lock = lock;
}
-
+
public boolean hasSuccess()
{
return m_success;
}
-
+
public void run ()
{
try
@@ -53,7 +53,7 @@
}
}
}
-
+
/**
* Worker thread that attempts to
* aquire a read lock. Start it, wait a little while
@@ -64,17 +64,17 @@
{
protected ReadWriteLock m_lock;
protected boolean m_success = false;
-
+
public TriesReadLock( final ReadWriteLock lock )
{
m_lock = lock;
}
-
+
public boolean hasSuccess()
{
return m_success;
}
-
+
public void run ()
{
try
@@ -89,11 +89,11 @@
}
}
}
-
+
public ReadWriteLockTestCase (String name) {
super (name);
}
-
+
/**
* Attempt to aquire and release read and write locks from
* different threads.
@@ -104,23 +104,23 @@
final ReadWriteLock lock = new ReadWriteLock();
final TriesWriteLock wl = new TriesWriteLock( lock );
final TriesReadLock rl = new TriesReadLock( lock );
-
+
rl.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire read lock.", rl.hasSuccess () );
-
+
wl.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire write lock.", !wl.hasSuccess () );
-
+
lock.release();
-
+
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire write lock after releasing read
lock.",
- wl.hasSuccess () );
-
+ wl.hasSuccess () );
+
lock.release();
-
+
//
// And see that the write lock is released properly.
//
@@ -128,10 +128,10 @@
r2.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire read lock.", r2.hasSuccess () );
-
+
lock.release();
}
-
+
/**
* Test that the lock throws an IllegalStateException when
* one attempts to release an already released lock.
@@ -149,7 +149,7 @@
// OK, we should receive this one.
}
}
-
+
/**
* Tests that attempts to aquire a write lock
* are given higher priority than attempts
@@ -162,23 +162,23 @@
TriesReadLock rlb = new TriesReadLock( lock );
TriesWriteLock wla = new TriesWriteLock( lock );
TriesWriteLock wlb = new TriesWriteLock( lock );
-
+
rla.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire read lock.", rla.hasSuccess () );
-
+
wla.start ();
wlb.start ();
-
+
//
// Give the write lock threads some time to attempt
// to aquire a lock.
//
Thread.currentThread().sleep( 100 );
-
+
rlb.start ();
Thread.currentThread().sleep( 100 );
-
+
//
// Two write locks queued up, one read lock queued up.
//
@@ -187,15 +187,15 @@
// rlb -> (has waiting write locks)
//
assertTrue( "Attempted to aquire write lock, and succeeded even though
it shouldn't be possible (rla has the lock).",
- !wla.hasSuccess() && !wlb.hasSuccess() &&
!rlb.hasSuccess() );
-
+ !wla.hasSuccess() && !wlb.hasSuccess() && !rlb.hasSuccess() );
+
//
// Upon releasing the lock, the write lock attempt should succeed,
while the read
// lock should still be waiting.
//
lock.release();
Thread.currentThread().sleep( 100 );
-
+
//
// One write lock queued up, one read lock queued up.
//
@@ -204,15 +204,15 @@
// rlb -> (has waiting write lock)
//
assertTrue( "Attempted to aquire write lock after releasing read
lock.",
- (wla.hasSuccess () || wlb.hasSuccess () ) &&
!rlb.hasSuccess ()
- && !(wla.hasSuccess () && wlb.hasSuccess () ));
-
+ (wla.hasSuccess () || wlb.hasSuccess () ) && !rlb.hasSuccess ()
+ && !(wla.hasSuccess () && wlb.hasSuccess () ));
+
//
// Release write lock again. The other one of wla and wlb should grab
the lock.
//
lock.release();
Thread.currentThread().sleep( 100 );
-
+
//
// Two write locks queued up, one read lock queued up.
//
@@ -220,17 +220,17 @@
// rlb -> (write lock is held)
//
assertTrue( "Attempted to aquire write lock after releasing read
lock.",
- wla.hasSuccess () && wlb.hasSuccess () && !rlb.hasSuccess
() );
-
+ wla.hasSuccess () && wlb.hasSuccess () && !rlb.hasSuccess () );
+
//
// Release the lock - the waiting read lock should grab it.
//
lock.release();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire write lock after releasing read
lock.",
- wla.hasSuccess () && wlb.hasSuccess () && rlb.hasSuccess
() );
+ wla.hasSuccess () && wlb.hasSuccess () && rlb.hasSuccess () );
}
-
+
/**
* Tests that the lock behaves correctly when
* multiple read locks are obtained.
@@ -241,28 +241,28 @@
TriesReadLock rla = new TriesReadLock( lock );
TriesReadLock rlb = new TriesReadLock( lock );
TriesWriteLock wla = new TriesWriteLock( lock );
-
+
rla.start ();
rlb.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Attempted to aquire read multiple read locks.",
- rla.hasSuccess () && rlb.hasSuccess () );
-
+ rla.hasSuccess () && rlb.hasSuccess () );
+
wla.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Write lock aquired even though read locks are held.",
!wla.hasSuccess () );
-
+
lock.release ();
Thread.currentThread().sleep( 100 );
assertTrue( "Write lock aquired even though read locks are held.
(There should be one read lock left)",
- !wla.hasSuccess () );
-
+ !wla.hasSuccess () );
+
lock.release ();
Thread.currentThread().sleep( 100 );
assertTrue( "Write lock not aquired even though lock should be
released.",
- wla.hasSuccess () );
+ wla.hasSuccess () );
}
-
+
/**
* Tests the tryAquireXXX methods.
*/
@@ -273,7 +273,7 @@
TriesReadLock rlb = new TriesReadLock( lock );
TriesWriteLock wla = new TriesWriteLock( lock );
TriesWriteLock wlb = new TriesWriteLock( lock );
-
+
//
// Grab a read lock, try to aquire one more (should work),
// and try aquiring a write lock (should not work).
@@ -281,18 +281,18 @@
rla.start ();
Thread.currentThread().sleep( 100 );
assertTrue( "Could not aquire a read lock.", rla.hasSuccess() );
-
+
assertTrue( "Could not aquire a read lock, even though only a read
lock is held.",
- lock.tryAquireRead() );
-
+ lock.tryAquireRead() );
+
assertTrue( "Could aquire a write lock.", !lock.tryAquireWrite() );
-
+
//
// Release both locks.
//
lock.release();
lock.release();
-
+
//
// Try aquiring a write lock (should work), a read
// lock (should fail) and another write lock (should fail).
@@ -300,14 +300,71 @@
assertTrue( "Could not aquire a write lock.", lock.tryAquireWrite() );
assertTrue( "Could aquire a read lock.", !lock.tryAquireRead() );
assertTrue( "Could aquire a write lock.", !lock.tryAquireWrite() );
-
+
//
// Release the write lock.
//
lock.release();
-
+
assertTrue( "Could not aquire a write lock after releasing the lock.",
- lock.tryAquireWrite() );
+ lock.tryAquireWrite() );
+ }
+
+ /**
+ * Tests a condition pointed out to me (L.Sutic) by Avi Drissman
+ * <a href="[EMAIL PROTECTED]">[EMAIL PROTECTED]</a>. If you hold
+ * a read lock, and a thread waiting for a write lock is interrupted,
+ * there is no way to aquire a read lock again.
+ *
+ * (This condition is fixed, 2001-10-31.)
+ */
+ public void testDeadLock () throws Exception {
+ ReadWriteLock lock = new ReadWriteLock();
+ TriesReadLock rla = new TriesReadLock( lock );
+ TriesReadLock rlb = new TriesReadLock( lock );
+ TriesWriteLock wla = new TriesWriteLock( lock );
+ TriesWriteLock wlb = new TriesWriteLock( lock );
+
+ //
+ // Grab a read lock.
+ //
+ rla.start();
+ Thread.currentThread().sleep( 100 );
+ assertTrue( rla.hasSuccess() );
+
+ //
+ // Try to grab a write lock. (The attempt stalls,
+ // because we are holding a read lock.)
+ //
+ wla.start();
+ Thread.currentThread().sleep( 100 );
+ assertTrue( !wla.hasSuccess() );
+
+ //
+ // Interupt the thread waiting for the write lock...
+ //
+ wla.interrupt();
+
+ //
+ // ...and release the read lock.
+ //
+ lock.release();
+
+ //
+ // Right, we are in the condition described by Drissman.
+ // Now try to aquire, in turn, a read and a write lock.
+ // Before the fix, the assertion immediately below
+ // would fail.
+ //
+ rlb.start();
+ Thread.currentThread().sleep( 100 );
+ assertTrue( rlb.hasSuccess() );
+ lock.release();
+
+ wlb.start();
+ Thread.currentThread().sleep( 100 );
+ assertTrue( wlb.hasSuccess() );
+ lock.release();
}
}
cvs server: Diffing src/test/org/apache/avalon/excalibur/datasource
cvs server: Diffing src/test/org/apache/avalon/excalibur/datasource/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/i18n
cvs server: Diffing src/test/org/apache/avalon/excalibur/i18n/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/io
cvs server: Diffing src/test/org/apache/avalon/excalibur/io/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/logger
cvs server: Diffing src/test/org/apache/avalon/excalibur/logger/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/monitor
cvs server: Diffing src/test/org/apache/avalon/excalibur/monitor/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming/memory
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming/memory/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming/rmi
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming/rmi/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/naming/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/pool
cvs server: Diffing src/test/org/apache/avalon/excalibur/pool/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/property
cvs server: Diffing src/test/org/apache/avalon/excalibur/property/test
cvs server: Diffing src/test/org/apache/avalon/excalibur/test
cvs server: Diffing src/xdocs
cvs server: Diffing src/xdocs/dtd
cvs server: Diffing src/xdocs/images
--
To unsubscribe, e-mail: <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>
