[kaffe] CVS kaffe (guilhem): GC Deadlock fixes for Linux 2.4/2.6 (at least).

Kaffe CVS cvs-commits at kaffe.org
Sun Jan 2 11:08:47 PST 2005


PatchSet 5747 
Date: 2005/01/02 19:03:30
Author: guilhem
Branch: HEAD
Tag: (none) 
Log:
GC Deadlock fixes for Linux 2.4/2.6 (at least).

        * kaffe/kaffevm/systems/unix-pthreads/lock-impl.c
        (setBlockState): Protect the mutex locking against signal.

        * kaffe/kaffevm/systems/unix-pthreads/thread-impl.c,
        kaffe/kaffevm/systems/unix-pthreads/thread-internal.h
        (KaffePThread_AckAndWaitForResume): New function.
        (suspend_handler): Call directly KaffePThread_AckAndWaitForResume
        now.
        (KaffePThread_getSuspendSignal): Export the signal number
        corresponding to the suspend signal.
        (jthread_suspendall): Cleant up the semaphore sweep. Added a few
        assertion checking.
        (jthread_unsuspendall): Added a few assertion checking.
        (tRun): Check whether the thread is killed when it is woken up.
        (jthread_exit): Do not cancel any thread as it may cause deadlocks
        on 2.4. Only interrupt and set the state to THREAD_KILL.

        * libraries/clib/native/Thread.c
        (VMThread.sleep): Check that if timeout is null we should return
        immediately.

Members: 
	ChangeLog:1.3291->1.3292 
	kaffe/kaffevm/systems/unix-jthreads/signal.c:1.29->1.30 
	kaffe/kaffevm/systems/unix-pthreads/lock-impl.c:1.14->1.15 
	kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.64->1.65 
	kaffe/kaffevm/systems/unix-pthreads/thread-internal.h:1.28->1.29 
	libraries/clib/native/Thread.c:1.21->1.22 

Index: kaffe/ChangeLog
diff -u kaffe/ChangeLog:1.3291 kaffe/ChangeLog:1.3292
--- kaffe/ChangeLog:1.3291	Sun Jan  2 05:21:39 2005
+++ kaffe/ChangeLog	Sun Jan  2 19:03:30 2005
@@ -1,3 +1,26 @@
+2005-01-02  Guilhem Lavaux  <guilhem at kaffe.org>
+
+	* kaffe/kaffevm/systems/unix-pthreads/lock-impl.c
+	(setBlockState): Protect the mutex locking against signal.
+	
+	* kaffe/kaffevm/systems/unix-pthreads/thread-impl.c,
+	kaffe/kaffevm/systems/unix-pthreads/thread-internal.h
+	(KaffePThread_AckAndWaitForResume): New function.
+	(suspend_handler): Call directly KaffePThread_AckAndWaitForResume
+	now.
+	(KaffePThread_getSuspendSignal): Export the signal number
+	corresponding to the suspend signal.
+	(jthread_suspendall): Cleant up the semaphore sweep. Added a few
+	assertion checking.
+	(jthread_unsuspendall): Added a few assertion checking.
+	(tRun): Check whether the thread is killed when it is woken up.
+	(jthread_exit): Do not cancel any thread as it may cause deadlocks
+	on 2.4. Only interrupt and set the state to THREAD_KILL.
+
+	* libraries/clib/native/Thread.c
+	(VMThread.sleep): Check that if timeout is null we should return
+	immediately.
+
 2005-01-02  Dalibor Topic  <robilad at kaffe.org>
 
 	* test/regression/TestSerializable2.java:
Index: kaffe/kaffe/kaffevm/systems/unix-jthreads/signal.c
diff -u kaffe/kaffe/kaffevm/systems/unix-jthreads/signal.c:1.29 kaffe/kaffe/kaffevm/systems/unix-jthreads/signal.c:1.30
--- kaffe/kaffe/kaffevm/systems/unix-jthreads/signal.c:1.29	Sat Jan  1 09:03:50 2005
+++ kaffe/kaffe/kaffevm/systems/unix-jthreads/signal.c	Sun Jan  2 19:03:33 2005
@@ -453,7 +453,7 @@
 	old_sigsegv = registerSyncSignalHandler(SIGSEGV, stackOverflowDetector);
 #endif
 #if defined(SIGBUS)
-	old_sigbus = haregisterSyncSignalHandler(SIGBUS, stackOverflowDetector);
+	old_sigbus = registerSyncSignalHandler(SIGBUS, stackOverflowDetector);
 #endif
 	
 	if (JTHREAD_SETJMP(outOfLoop) == 0)
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/lock-impl.c
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/lock-impl.c:1.14 kaffe/kaffe/kaffevm/systems/unix-pthreads/lock-impl.c:1.15
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/lock-impl.c:1.14	Mon Dec 20 02:29:52 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/lock-impl.c	Sun Jan  2 19:03:33 2005
@@ -19,18 +19,37 @@
 #ifdef KAFFE_BOEHM_GC
 #include "boehm-gc/boehm/include/gc.h"
 #endif
+#include <signal.h>
 
 static inline void
-setBlockState(jthread_t cur, unsigned int newState, void *sp)
+setBlockState(jthread_t cur, unsigned int newState, void *sp, sigset_t *old_mask)
 {
+  int suspendSig;
+  sigset_t suspendMask, pendingMask;
+
+  suspendSig = KaffePThread_getSuspendSignal();
+  sigemptyset(&suspendMask);
+  sigaddset(&suspendMask, suspendSig);
+  pthread_sigmask(SIG_BLOCK, &suspendMask, old_mask);
+
   pthread_mutex_lock(&cur->suspendLock);
   cur->blockState |= newState;
   cur->stackCur  = sp;
   pthread_mutex_unlock(&cur->suspendLock);
+
+  /* This thread is protected against suspendall. So if a signal has been
+   * received it is just before the mutex_lock and after pthread_sigmask.
+   * We must acknowledge and go in WaitForResume.
+   */
+  sigpending(&pendingMask); 
+  if (sigismember(&pendingMask, suspendSig))
+  {
+    KaffePThread_AckAndWaitForResume(cur, newState);
+  }
 }
 
 static inline void
-clearBlockState(jthread_t cur, unsigned int newState)
+clearBlockState(jthread_t cur, unsigned int newState, sigset_t *old_mask)
 {
   pthread_mutex_lock(&cur->suspendLock);
   cur->blockState &= ~newState;
@@ -38,7 +57,7 @@
     {
       DBG(JTHREADDETAIL, dprintf("Changing blockstate of %p to %d while in suspend, block again\n",  cur, newState));
 
-      KaffePThread_WaitForResume(true);
+      KaffePThread_WaitForResume(true, 0);
     }
   else
     {
@@ -51,16 +70,22 @@
    */
   if (cur->status == THREAD_KILL)
     pthread_exit(NULL);
+
+  pthread_sigmask(SIG_SETMASK, old_mask, NULL);
+  /* Here the state is not SS_PENDING_SUSPEND so releasing the signal will
+   * not trigger a deadlock.
+   */
 }
 
 void
 jmutex_lock( jmutex* lk )
 {
   jthread_t cur = jthread_current ();
+  sigset_t oldmask;
 
-  setBlockState(cur, BS_MUTEX, (void*)&cur);
+  setBlockState(cur, BS_MUTEX, (void*)&cur, &oldmask);
   pthread_mutex_lock( lk );
-  clearBlockState(cur, BS_MUTEX);
+  clearBlockState(cur, BS_MUTEX, &oldmask);
 }
 
 
@@ -68,10 +93,11 @@
 ThreadCondWait(jthread_t cur, jcondvar *cv, jmutex *mux)
 {
   int status;
+  sigset_t oldmask;
 
-  setBlockState(cur, BS_CV, (void*)&cur);
+  setBlockState(cur, BS_CV, (void*)&cur, &oldmask);
   status = pthread_cond_wait( cv, mux );
-  clearBlockState(cur, BS_CV);
+  clearBlockState(cur, BS_CV, &oldmask);
 
   return status;
 }
@@ -87,6 +113,7 @@
   int             status;
   struct timespec abst;
   struct timeval  now;
+  sigset_t oldmask;
 
   /*
    * If a thread trying to get a heavy lock is interrupted, we may get here
@@ -120,9 +147,9 @@
 	      abst.tv_nsec -= 1000000000;
 	    }
 
-	  setBlockState(cur, BS_CV_TO, (void*)&cur);
+	  setBlockState(cur, BS_CV_TO, (void*)&cur, &oldmask);
 	  status = pthread_cond_timedwait( cv, mux, &abst);
-	  clearBlockState(cur, BS_CV_TO);
+	  clearBlockState(cur, BS_CV_TO, &oldmask);
 	}
     }
 
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.64 kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.65
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.64	Wed Dec 29 16:27:43 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c	Sun Jan  2 19:03:33 2005
@@ -784,6 +784,8 @@
 	/* Wait until we get re-used (by TcreateThread). No need to update the
 	 * blockState, since we aren't active anymore */
 	repsem_wait( &cur->sem);
+	if (cur->status == THREAD_KILL)
+	  break;
 
 	/*
 	 * we have already been moved back to the activeThreads list by
@@ -1024,7 +1026,7 @@
 	   */
 	  for ( t=cache; t != NULL; t = t->next ){
 		t->status = THREAD_KILL;
-		pthread_cancel( t->tid);
+		repsem_post(&t->sem);
 	  }
 
 	  for ( t=activeThreads; t != NULL; t = t->next ){
@@ -1033,8 +1035,6 @@
 		  t->status = THREAD_KILL;
 		  /* Send an interrupt event to the remote thread. */
 		  jthread_interrupt(t);
-		  /* Cancel it to be sure it is dead. */
-		  pthread_cancel( t->tid);
 		}
 	  }
 
@@ -1156,7 +1156,7 @@
  * mask and to release the suspendLock mutex only after this. In that case we
  * keep in sync with jthread_suspendall()
  */
-void KaffePThread_WaitForResume(int releaseMutex)
+void KaffePThread_WaitForResume(int releaseMutex, unsigned int state)
 {
   volatile jthread_t cur = jthread_current();
   int s;
@@ -1164,8 +1164,16 @@
 
   if (releaseMutex)
     {
-      pthread_sigmask(SIG_SETMASK, &suspendSet, &oldset);
+      pthread_sigmask(SIG_BLOCK, &suspendSet, &oldset);
       pthread_mutex_unlock(&cur->suspendLock);
+      /*
+       * In that particular case we have to release the mutex on the thread list
+       * because it may cause deadlocks in the GC thread. It does not hurt as we
+       * do not access the thread list but we will have to reacquire it before
+       * returning.
+       */
+      if (cur->blockState & BS_THREAD)
+	      pthread_mutex_unlock(&activeThreadsLock);
     }
 
   /* freeze until we get a subsequent sigResume */
@@ -1173,15 +1181,22 @@
     sigwait( &suspendSet, &s);
   
   DBG( JTHREADDETAIL, dprintf("sigwait return: %p\n", cur));
-    
-  cur->stackCur     = NULL;
+
+  /* If the thread needs to be put back in a block state
+   * we must not reset the stack pointer.
+   */
+  if (state == 0)
+    cur->stackCur     = NULL;
   cur->suspendState = 0;
+  cur->blockState |= state;
   
   /* notify the critSect owner we are leaving the handler */
   repsem_post( &critSem);
 
   if (releaseMutex)
     {
+      if (cur->blockState & BS_THREAD)
+	pthread_mutex_lock(&activeThreadsLock);
       pthread_sigmask(SIG_SETMASK, &oldset, NULL);
     }
 }
@@ -1206,26 +1221,33 @@
   if ( !cur || !cur->active )
 	return;
 
-  if ( cur->suspendState == SS_PENDING_SUSPEND ){
-	JTHREAD_JMPBUF env;
-
-	/*
-	 * Note: We're not gonna do a longjmp to this place, we just want
-	 * to do something that will save all of the registers onto the stack.
-	 */
-	JTHREAD_SETJMP(env);
-
-	/* assuming we are executing on the thread stack, we record our current pos */
-	cur->stackCur     = (void*)&env;
-	cur->suspendState = SS_SUSPENDED;
-
-	/* notify the critSect owner that we are now suspending in the handler */
-	repsem_post( &critSem);
+  KaffePThread_AckAndWaitForResume(cur, 0);
+}
 
-	KaffePThread_WaitForResume(false);
+void KaffePThread_AckAndWaitForResume(volatile jthread_t cur, unsigned int state)
+{
+  if ( cur->suspendState == SS_PENDING_SUSPEND ){
+    JTHREAD_JMPBUF env;
+    
+    /*
+     * Note: We're not gonna do a longjmp to this place, we just want
+     * to do something that will save all of the registers onto the stack.
+     */
+    JTHREAD_SETJMP(env);
+    
+    /* assuming we are executing on the thread stack, we record our current pos */
+    cur->stackCur     = (void*)&env;
+    cur->suspendState = SS_SUSPENDED;
+    cur->blockState  &= ~state;
+    
+    /* notify the critSect owner that we are now suspending in the handler */
+    repsem_post( &critSem);
+    
+    KaffePThread_WaitForResume(false, state);
   }
 }
 
+
 /**
  * The resume signal handler, which we mainly need to get the implicit sigreturn
  * call (i.e. to unblock a preceeding sigwait).
@@ -1312,18 +1334,8 @@
 	 */
 	while (numPending > 0)
 	  {
-	    repsem_getvalue(&critSem, &val);
 	    repsem_wait( &critSem);
-	    for ( numPending=0,t=activeThreads; t; t = t->next ){
-	      if (t->suspendState == SS_PENDING_SUSPEND)
-		numPending++;
-	    }
-	  }
-	/* Now flush the semaphore. */
-	repsem_getvalue(&critSem, &val);
-	for (;val != 0; val--)
-	  {
-	    repsem_wait(&critSem);
+	    numPending--;
 	  }
 
 #else
@@ -1362,11 +1374,14 @@
 	return;
 
   if ( --critSection == 0 ){
+	  int val;
 	/* No need to sync, there's nobody else running. However it seems
 	 * we cannot use mutexes as they cause a deadlock when the world
 	 * is suspended.
 	 */
-	  protectThreadList(cur);
+	protectThreadList(cur);
+	repsem_getvalue(&critSem, &val);
+	assert(val == 0);
 
 #if !defined(KAFFE_BOEHM_GC)
 	for ( t=activeThreads; t; t = t->next ){
@@ -1408,6 +1423,8 @@
 	GC_start_world();
 
 #endif
+	repsem_getvalue(&critSem, &val);
+	assert(val == 0);
 	  unprotectThreadList(cur);
 
   }
@@ -1592,4 +1609,9 @@
 void jthread_atexit(void (* func)(void))
 {
   runOnExit = func;
+}
+
+int KaffePThread_getSuspendSignal()
+{
+  return sigSuspend;
 }
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-internal.h
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-internal.h:1.28 kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-internal.h:1.29
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-internal.h:1.28	Sat Dec 25 19:09:17 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-internal.h	Sun Jan  2 19:03:34 2005
@@ -414,7 +414,9 @@
 
 int jthread_interrupted(jthread_t jt);
 
-void KaffePThread_WaitForResume(int releaseMutex);
+void KaffePThread_WaitForResume(int releaseMutex, unsigned int state);
+void KaffePThread_AckAndWaitForResume(jthread_t cur, unsigned int newState);
+int KaffePThread_getSuspendSignal();
 
 void detectStackBoundaries(jthread_t jtid, int mainThreadStackSize);
 
Index: kaffe/libraries/clib/native/Thread.c
diff -u kaffe/libraries/clib/native/Thread.c:1.21 kaffe/libraries/clib/native/Thread.c:1.22
--- kaffe/libraries/clib/native/Thread.c:1.21	Sun Dec 19 06:25:12 2004
+++ kaffe/libraries/clib/native/Thread.c	Sun Jan  2 19:03:34 2005
@@ -82,6 +82,10 @@
 {
   jthread_t	cur = KTHREAD(current)();
 
+  /* Temporary hack waiting for the Classpath merge. */
+  if (timeout == 0)
+    return;
+
   if(KTHREAD(interrupted)(cur))
     {
       throwException(InterruptedException);




More information about the kaffe mailing list