This is the mail archive of the java-patches@sourceware.cygnus.com mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

PATCH: Threads and SIGINT


Here's a new patch for threads and SIGINT handling. This is a somewhat refined
version of the threads patch that I have previously posted.

Basically it:

  - fixes miscellaneous bugs related to join() and interrupt().
  - allows interrupt() to interrupt threads that are in wait(), on Linux, using
the interrupt signal. This technique is unfortunately non portable (doesn't work
on Solaris) so there's a few #ifdefs in there. But it allows us to pass all the
threads stress tests I've come across so far.
  - fixes libgcj to exit cleanly and correctly on ^C by removing explicit SIGINT
handling and fixing an associated deadlock in boehm-gc.

I have tested these fairly extensively.

I'm going to be away for a week or two over Christmas/New Year and I'd really
like to get this stuff checked in before I go. Tom: can you look at this ASAP,
please?

Regarding the Solaris thread problems reported by Joerg Brunsmann: I have
reproduced this on my Solaris x86 installation, and I'm convinced that these
problems are in fact due to a bug in Solaris rather than anything wrong with our
code. Solaris 7 (and I presume some 2.6 installations as well, since Joerg
reports that he is using 2.6) have a broken recursive mutex implementation,
resulting in calls to pthread_cond_wait never returning because it cannot
reacquire the mutex.

More information about the Solaris bug here:
http://x23.deja.com/getdoc.xp?AN=559993012&CONTEXT=945928882.1764425735&hitnum=1

 "There is an (escalated) Solaris 7 bug report about this problem:

   Bugid: 4288299
   Category: library
   Subcategory: libthread
   Synopsis: Recursive mutexes are not properly released

 The bug report discusses the deadlock problem with
 PTHREAD_MUTEX_RECURSIVE, but the same problem exists with
 PTHREAD_MUTEX_ERRORCHECK."

In order to fix this I think we need to force the use of default, non-recursive
mutexes on Solaris. I think this could be done with something like "#if defined
(SOLARIS && HAVE_PTHREAD_MUTEX_SETTYPE) #undef HAVE_PTHREAD_MUTEX_SETTYPE" at
the top of posix-threads.h, but perhaps there is a better way.



1999-12-22  Bryce McKinlay  <bryce@albatross.co.nz>

        * java/lang/natObject.cc (notify): Throw message with
        IllegalMonitorStateException.
        (notifyAll): Ditto.
        (wait): Ditto.
        * java/lang/Thread.java (isInterrupted): Don't clear interrupt_flag.
        (isInterrupted_): New function, which does clear interrupt_flag.
        (interrupt): Use `isInterrupted_'.
        * java/lang/natThread.cc (interrupt): Don't call _Jv_CondNotify on
        Linux.
        (join): Set `prev' in joiner loop.
        Change various calls to `isInterrupted' to use `isInterrupted_'.
        * posix-threads.cc (_Jv_CondWait): Allways use pthread_cond_timedwait
        on linux. Set result to 0 on an interrupt. Test interrupted status
        of jthread object directly.
        FLAG_INTERRUPTED: removed.
        (_Jv_ThreadStart): Throw OutOfMemoryError if pthread_create fails.
        (_Jv_ThreadInterrupt): Don't set FLAG_INTERRUPTED.
        (_Jv_InitThreads): Don't block SIGINT.
        (_Jv_ThreadWait): Don't configure SIGINT handler.
        (_Jv_ThreadInitData): Set `jthread'.
        * include/posix-threads.h (_Jv_Thread_t): jthread: new field.

1999-12-22  Bryce McKinlay  <bryce@albatross.co.nz>

        * linux_threads.c: Don't block SIGINT, SIGQUIT, SIGTERM in the
        NO_SIGNALS case.

Index: boehm-gc/linux_threads.c
===================================================================
RCS file: /cvs/java/libgcj/boehm-gc/linux_threads.c,v
retrieving revision 1.4
diff -u -r1.4 linux_threads.c
--- linux_threads.c	1999/11/01 23:15:51	1.4
+++ linux_threads.c	1999/12/23 06:27:37
@@ -173,6 +173,11 @@
     /* is no race.						*/
     if (sigfillset(&mask) != 0) ABORT("sigfillset() failed");
     if (sigdelset(&mask, SIG_RESTART) != 0) ABORT("sigdelset() failed");
+#ifdef NO_SIGNALS
+    if (sigdelset(&mask, SIGINT) != 0) ABORT("sigdelset() failed");
+    if (sigdelset(&mask, SIGQUIT) != 0) ABORT("sigdelset() failed");
+    if (sigdelset(&mask, SIGTERM) != 0) ABORT("sigdelset() failed");
+#endif
     do {
 	    me->signal = 0;
 	    sigsuspend(&mask);             /* Wait for signal */
@@ -433,6 +438,21 @@
     if (sigfillset(&act.sa_mask) != 0) {
     	ABORT("sigfillset() failed");
     }
+
+#ifdef NO_SIGNALS
+    if (sigdelset(&act.sa_mask, SIGINT) != 0) {
+       ABORT("sigdelset() failed");
+    }
+
+    if (sigdelset(&act.sa_mask, SIGQUIT) != 0) {
+       ABORT("sigdelset() failed");
+    }
+
+    if (sigdelset(&act.sa_mask, SIGTERM) != 0) {
+       ABORT("sigdelset() failed");
+    }
+#endif
+
     /* SIG_RESTART is unmasked by the handler when necessary. 	*/
     act.sa_handler = GC_suspend_handler;
     if (sigaction(SIG_SUSPEND, &act, NULL) != 0) {
Index: libjava/java/lang/Thread.java
===================================================================
RCS file: /cvs/java/libgcj/libjava/java/lang/Thread.java,v
retrieving revision 1.2
diff -u -r1.2 Thread.java
--- Thread.java	1999/11/04 16:45:11	1.2
+++ Thread.java	1999/12/23 06:27:37
@@ -73,15 +73,15 @@
 
   public static boolean interrupted ()
   {
-    return currentThread().isInterrupted();
+    return currentThread().isInterrupted_();
   }
 
   // FIXME: it seems to me that this should be synchronized.
+  // Check the threads interrupted status. Note that this does not clear the
+  // threads interrupted status (per JDK 1.2 online API documentation).
   public boolean isInterrupted ()
   {
-    boolean r = interrupt_flag;
-    interrupt_flag = false;
-    return r;
+    return interrupt_flag;
   }
 
   public final boolean isAlive ()
@@ -112,6 +112,15 @@
   // This method exists only to avoid a warning from the C++ compiler.
   private static final native void run__ (Object obj);
   private native final void finish_ ();
+
+  // Convenience method to check and clear the thread's interrupted status.  
+  private boolean isInterrupted_ ()
+  {
+    boolean r = interrupt_flag;
+    interrupt_flag = false;
+    return r;
+  }
+  
   private final void run_ ()
   {
     try
Index: libjava/java/lang/natThread.cc
===================================================================
RCS file: /cvs/java/libgcj/libjava/java/lang/natThread.cc,v
retrieving revision 1.8
diff -u -r1.8 natThread.cc
--- natThread.cc	1999/11/05 20:59:40	1.8
+++ natThread.cc	1999/12/23 06:27:38
@@ -130,10 +130,16 @@
   // another thread to exit.
   natThread *nt = (natThread *) data;
   _Jv_MutexLock (&nt->interrupt_mutex);
+  // Send a signal to the target thread to interrupt system calls. On Linux,
+  // this will also interrupt the target thread from *any* _Jv_CondWait call.
+  // This behaviour is not portable, however, so other platforms must notify 
+  // condition variable (which won't work if the thread is in wait() and 
+  // therefore not waiting for the right condition).
+  _Jv_ThreadInterrupt (nt->thread);
+#ifndef LINUX_THREADS
   _Jv_CondNotify (&nt->interrupt_cond, &nt->interrupt_mutex);
+#endif
   _Jv_MutexUnlock (&nt->interrupt_mutex);
-
-  _Jv_ThreadInterrupt (nt->thread);
 }
 
 void
@@ -145,7 +151,7 @@
     _Jv_Throw (new IllegalArgumentException);
 
   Thread *current = currentThread ();
-  if (current->isInterrupted ())
+  if (current->isInterrupted_ ())
     _Jv_Throw (new InterruptedException);
 
   // Update the list of all threads waiting for this thread to exit.
@@ -199,11 +205,12 @@
 	  t->next = 0;
 	  break;
 	}
+      prev = t;
     }
   JvAssert (t != NULL);
   _Jv_MonitorExit (this);
 
-  if (current->isInterrupted ())
+  if (current->isInterrupted_ ())
     _Jv_Throw (new InterruptedException);
 }
 
@@ -240,7 +247,7 @@
     ++nanos;
 
   Thread *current = currentThread ();
-  if (current->isInterrupted ())
+  if (current->isInterrupted_ ())
     _Jv_Throw (new InterruptedException);
 
   // We use a condition variable to implement sleeping so that an
@@ -253,7 +260,7 @@
 		  millis, nanos);
   }
 
-  if (current->isInterrupted ())
+  if (current->isInterrupted_ ())
     _Jv_Throw (new InterruptedException);
 }
 
Index: libjava/java/lang/natObject.cc
===================================================================
RCS file: /cvs/java/libgcj/libjava/java/lang/natObject.cc,v
retrieving revision 1.4
diff -u -r1.4 natObject.cc
--- natObject.cc	1999/11/25 00:36:51	1.4
+++ natObject.cc	1999/12/23 06:27:38
@@ -175,7 +175,8 @@
     sync_init ();
   _Jv_SyncInfo *si = (_Jv_SyncInfo *) sync_info;
   if (_Jv_CondNotify (&si->condition, &si->mutex))
-    JvThrow (new IllegalMonitorStateException);
+    JvThrow (new IllegalMonitorStateException(JvNewStringLatin1 
+                                              ("current thread not owner")));
 }
 
 void
@@ -185,7 +186,8 @@
     sync_init ();
   _Jv_SyncInfo *si = (_Jv_SyncInfo *) sync_info;
   if (_Jv_CondNotifyAll (&si->condition, &si->mutex))
-    JvThrow (new IllegalMonitorStateException);
+    JvThrow (new IllegalMonitorStateException(JvNewStringLatin1 
+                                              ("current thread not owner")));
 }
 
 void
@@ -197,7 +199,8 @@
     JvThrow (new IllegalArgumentException);
   _Jv_SyncInfo *si = (_Jv_SyncInfo *) sync_info;
   if (_Jv_CondWait (&si->condition, &si->mutex, timeout, nanos))
-    JvThrow (new IllegalMonitorStateException);
+    JvThrow (new IllegalMonitorStateException(JvNewStringLatin1 
+                                              ("current thread not owner")));
   if (Thread::interrupted())
     JvThrow (new InterruptedException);
 }
Index: libjava/posix-threads.cc
===================================================================
RCS file: /cvs/java/libgcj/libjava/posix-threads.cc,v
retrieving revision 1.14
diff -u -r1.14 posix-threads.cc
--- posix-threads.cc	1999/11/30 18:53:15	1.14
+++ posix-threads.cc	1999/12/23 06:27:38
@@ -27,11 +27,14 @@
 #include <time.h>
 #include <signal.h>
 #include <errno.h>
+#include <limits.h>
 
 #include <gcj/cni.h>
 #include <jvm.h>
 #include <java/lang/Thread.h>
 #include <java/lang/System.h>
+#include <java/lang/Long.h>
+#include <java/lang/OutOfMemoryError.h>
 
 // This is used to implement thread startup.
 struct starter
@@ -58,7 +61,7 @@
 
 // The signal to use when interrupting a thread.
 #ifdef LINUX_THREADS
-  // LinuxThreads usurps both SIGUSR1 and SIGUSR2.
+  // LinuxThreads (prior to glibc 2.1) usurps both SIGUSR1 and SIGUSR2.
 #  define INTR SIGHUP
 #else /* LINUX_THREADS */
 #  define INTR SIGUSR2
@@ -72,8 +75,6 @@
 #define FLAG_START   0x01
 // Thread is daemon.
 #define FLAG_DAEMON  0x02
-// Thread was interrupted by _Jv_ThreadInterrupt.
-#define FLAG_INTERRUPTED  0x04
 
 
 
@@ -86,58 +87,82 @@
 
   int r;
   pthread_mutex_t *pmu = _Jv_PthreadGetMutex (mu);
-  struct timespec ts; 
+  struct timespec ts;
   jlong m, m2, startTime;
   bool done_sleeping = false;
 
   if (millis == 0 && nanos == 0)
-    r = pthread_cond_wait (cv, pmu);
+    {
+#ifdef LINUX_THREADS
+      // pthread_cond_timedwait can be interrupted by a signal on linux, while
+      // pthread_cond_wait can not. So pthread_cond_timedwait() forever.
+      m = java::lang::Long::MAX_VALUE;
+      ts.tv_sec = LONG_MAX;
+      ts.tv_nsec = 0;
+#endif
+    }
   else
     {
       startTime = java::lang::System::currentTimeMillis();
       m = millis + startTime;
+      ts.tv_sec = m / 1000; 
+      ts.tv_nsec = ((m % 1000) * 1000000) + nanos; 
+    }
 
-      do
-	{  
-	  ts.tv_sec = m / 1000; 
-	  ts.tv_nsec = ((m % 1000) * 1000000) + nanos; 
+  _Jv_Thread_t *current = _Jv_ThreadCurrentData();
 
+  do
+    {
+      r = EINTR;
+      // Check to ensure the thread hasn't already been interrupted.
+      if (!(current->jthread->isInterrupted ()))
+        {
+#ifdef LINUX_THREADS	
+	  // FIXME: in theory, interrupt() could be called on this thread
+	  // between the test above and the wait below, resulting in the 
+	  // interupt() call failing. I don't see a way to fix this 
+	  // without significant changes to the implementation.
 	  r = pthread_cond_timedwait (cv, pmu, &ts);
-
-	  if (r == EINTR)
+#else
+	  if (millis == 0 && nanos == 0)
+	    r = pthread_cond_wait (cv, pmu);
+	  else	  
+	    r = pthread_cond_timedwait (cv, pmu, &ts);	  
+#endif
+	}
+      
+      if (r == EINTR)
+	{
+	  /* We were interrupted by a signal.  Either this is
+	     because we were interrupted intentionally (i.e. by
+	     Thread.interrupt()) or by the GC if it is
+	     signal-based.  */
+	  if (current->jthread->isInterrupted ())
 	    {
-	      /* We were interrupted by a signal.  Either this is
-		 because we were interrupted intentionally (i.e. by
-		 Thread.interrupt()) or by the GC if it is
-		 signal-based.  */
-	      _Jv_Thread_t *current = _Jv_ThreadCurrentData();
-	      if (current->flags & FLAG_INTERRUPTED)
+	      r = 0;
+              done_sleeping = true;
+            }
+	  else
+            {
+	      /* We were woken up by the GC or another signal.  */
+	      m2 = java::lang::System::currentTimeMillis ();
+	      if (m2 >= m)
 		{
-                  current->flags &= ~(FLAG_INTERRUPTED);
-                  done_sleeping = true;
-                }
-	      else
-                {
-		  /* We were woken up by the GC or another signal.  */
-		  m2 = java::lang::System::currentTimeMillis ();
-		  if (m2 >= m)
-		    {
-		      r = 0;
-		      done_sleeping = true;
-		    }
+		  r = 0;
+		  done_sleeping = true;
 		}
 	    }
-	  else if (r == ETIMEDOUT)
-	    {
-	      /* A timeout is a normal result.  */
-	      r = 0;
-	      done_sleeping = true;
-	    }
-	  else
-	    done_sleeping = true;
 	}
-      while (! done_sleeping);
+      else if (r == ETIMEDOUT)
+	{
+	  /* A timeout is a normal result.  */
+	  r = 0;
+	  done_sleeping = true;
+	}
+      else
+	done_sleeping = true;
     }
+  while (! done_sleeping);
 
   return r != 0;
 }
@@ -272,20 +297,13 @@
   sigemptyset (&act.sa_mask);
   act.sa_flags = 0;
   sigaction (INTR, &act, NULL);
-
-  // Arrange for SIGINT to be blocked to all threads.  It is only
-  // deliverable to the master thread.
-  sigset_t mask;
-  sigemptyset (&mask);
-  sigaddset (&mask, SIGINT);
-  pthread_sigmask (SIG_BLOCK, &mask, NULL);
 }
 
 void
-_Jv_ThreadInitData (_Jv_Thread_t **data, java::lang::Thread *)
+_Jv_ThreadInitData (_Jv_Thread_t **data, java::lang::Thread *jthread)
 {
   _Jv_Thread_t *info = new _Jv_Thread_t;
-
+  info->jthread = jthread;
   info->flags = 0;
 
   // FIXME register a finalizer for INFO here.
@@ -361,20 +379,20 @@
     }
   else
     data->flags |= FLAG_DAEMON;
-  pthread_create (&data->thread, &attr, really_start, (void *) info);
-
+  int r = pthread_create (&data->thread, &attr, really_start, (void *) info);
+  
   pthread_attr_destroy (&attr);
+
+  if (r)
+    {
+      const char* msg = "Cannot create additional threads";
+      JvThrow (new java::lang::OutOfMemoryError (JvNewStringUTF (msg)));
+    }
 }
 
 void
 _Jv_ThreadWait (void)
 {
-  // Arrange for SIGINT to be delivered to the master thread.
-  sigset_t mask;
-  sigemptyset (&mask);
-  sigaddset (&mask, SIGINT);
-  pthread_sigmask (SIG_UNBLOCK, &mask, NULL);
-
   pthread_mutex_lock (&daemon_mutex);
   if (non_daemon_count)
     pthread_cond_wait (&daemon_cond, &daemon_mutex);
@@ -384,6 +402,5 @@
 void
 _Jv_ThreadInterrupt (_Jv_Thread_t *data)
 {
-  data->flags |= FLAG_INTERRUPTED; 
   pthread_kill (data->thread, INTR);
 }
Index: libjava/include/posix-threads.h
===================================================================
RCS file: /cvs/java/libgcj/libjava/include/posix-threads.h,v
retrieving revision 1.10
diff -u -r1.10 posix-threads.h
--- posix-threads.h	1999/12/09 16:57:27	1.10
+++ posix-threads.h	1999/12/23 06:27:38
@@ -73,6 +73,7 @@
 {
   // Flag values are defined in implementation.
   int flags;
+  java::lang::Thread *jthread;
 
   // Actual thread id.
   pthread_t thread;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]