Re: known synchronized() failures?

[Bryce McKinlay <mckinlay at redhat dot com>]

Thomas Aeby wrote:

> On Mon, 2005-05-02 at 14:37 +0200, Thomas Aeby wrote:
>  
>
> > typical stack trace of the process consuming all the CPU (and is the
> > one who is trying to get a lock) looks like
> >
> > /disk/hdc2/glibc/debian-build/glibc_2.3.2.ds1-20.test2/glibc-2.3.2.ds1/build-tree/glibc-2.3.2/linuxthreads/restart.h:24
> > /disk/hdc2/glibc/debian-build/glibc_2.3.2.ds1-20.test2/glibc-2.3.2.ds1/build-tree/glibc-2.3.2/linuxthreads/mutex.c:199
> > ../../../gcc-4.0.0/libjava/posix-threads.cc:192
> > ../../../gcc-4.0.0/libjava/java/lang/natObject.cc:929
> >    
>
> Oh, and as I look at posix-threads.cc: I might have to add that the
> thread actually being blocked at the beginning of the synchronized() {}
> block is actually in "interrupted" state (don't know if it is from the
> start since the idea behind is that the thread starts a few sub threads
> which will interrupt() after a while) ... hmh, I can't say what
> _Jv_CondWait() is supposed to do in this case
>
>  

OK, I think you are on to something here. _Jv_CondWait was designed to 
implement the Java interrupt semantics for things like Object.wait() and 
Thread.join(). However, MonitorEnter/MonitorExit are not interruptable 
and should ignore the interrupt flag completely. _Jv_MonitorEnter uses 
condition variables to implement lightweight->heavyweight lock 
inflation, but because it uses _Jv_CondWait, it can be interrupted when 
it shouldn't be. Of course, _Jv_MonitorEnter does not check for 
interruption so we just get stuck in a loop.

Here's a lame (untested) patch that should work around the problem. Can 
you try this out and see if it works for you? A better solution might be 
to implement a "light" version of _Jv_CondWait that is not interruptable.

Bryce



Index: java/lang/natObject.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/java/lang/natObject.cc,v
retrieving revision 1.33
diff -u -r1.33 natObject.cc
--- java/lang/natObject.cc	5 Jan 2005 05:09:09 -0000	1.33
+++ java/lang/natObject.cc	4 May 2005 00:17:32 -0000
@@ -35,6 +35,8 @@
 
 
 
+using namespace java::lang;
+
 // This is used to represent synchronization information.
 struct _Jv_SyncInfo
 {
@@ -926,12 +928,20 @@
 	  release_set(&(he -> address), (address | REQUEST_CONVERSION | HEAVY));
 				// release lock on he
 	  LOG(REQ_CONV, (address | REQUEST_CONVERSION | HEAVY), self);
+	  jboolean flag = false;
 	  while ((he -> address & ~FLAGS) == (address & ~FLAGS))
 	    {
 	      // Once converted, the lock has to retain heavyweight
-	      // status, since heavy_count > 0 . 
-	      _Jv_CondWait (&(hl->si.condition), &(hl->si.mutex), 0, 0);
+	      // status, since heavy_count > 0.
+	      int r = _Jv_CondWait (&(hl->si.condition), &(hl->si.mutex), 0, 0);
+	      if (r == _JV_INTERRUPTED)
+	        {
+		  flag = true;
+		  Thread::currentThread()->interrupt_flag = false;
+		}
 	    }
+	  if (flag)
+	    Thread::currentThread()->interrupt_flag = flag;
 	  keep_live(addr);
 		// Guarantee that hl doesn't get unlinked by finalizer.
 		// This is only an issue if the client fails to release