[kaffe] CVS kaffe (guilhem): Fixlets. Brought back protections to highlight SMP bug.

Kaffe CVS cvs-commits at kaffe.org
Wed Dec 29 08:32:06 PST 2004


PatchSet 5731 
Date: 2004/12/29 16:27:39
Author: guilhem
Branch: HEAD
Tag: (none) 
Log:
Fixlets. Brought back protections to highlight SMP bug.

        * kaffe/kaffevm/gtypes.h: New intp type for signed integer that may
        represent pointers.

        * kaffe/kaffevm/thread.c
        (nameThread): Check that the thread has been linked before returning
        the name.

        * kaffe/kaffevm/kaffe-gc/gc-mem.c
        (page_alloc): Use intp instead of intptr_t as it is not available on
        all systems.

        * kaffe/kaffevm/kaffe-gc/gc-refs.c
        (TwalkLiveThreads): Reuse jthread_walkLiveThreads_r to highlight the
        bug on SMP.

        * kaffe/kaffevm/systems/unix-jthreads/jsignal.h: Removed ignoreSignal.

        * kaffe/kaffevm/systems/unix-pthreads/syscalls.c
        (jthreadedForkExec): Save the old sigmask and restore it after fork.

        * kaffe/kaffevm/systems/unix-pthreads/thread-impl.c
        (jthread_create): Cleanup the thread queue if pthread_create fails.
        (jthread_suspendall): Temporarily changed logic to insure that all
        concerned threads receive the signal.

Members: 
	ChangeLog:1.3275->1.3276 
	kaffe/kaffevm/gtypes.h:1.14->1.15 
	kaffe/kaffevm/thread.c:1.89->1.90 
	kaffe/kaffevm/kaffe-gc/gc-mem.c:1.22->1.23 
	kaffe/kaffevm/kaffe-gc/gc-refs.c:1.9->1.10 
	kaffe/kaffevm/systems/unix-jthreads/jsignal.h:1.5->1.6 
	kaffe/kaffevm/systems/unix-pthreads/jsignal.h:1.4->1.5 
	kaffe/kaffevm/systems/unix-pthreads/signal.c:1.16->1.17 
	kaffe/kaffevm/systems/unix-pthreads/syscalls.c:1.25->1.26 
	kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.63->1.64 

Index: kaffe/ChangeLog
diff -u kaffe/ChangeLog:1.3275 kaffe/ChangeLog:1.3276
--- kaffe/ChangeLog:1.3275	Wed Dec 29 15:47:25 2004
+++ kaffe/ChangeLog	Wed Dec 29 16:27:39 2004
@@ -1,5 +1,32 @@
 2004-12-29  Guilhem Lavaux  <guilhem at kaffe.org>
 
+	* kaffe/kaffevm/gtypes.h: New intp type for signed integer that may
+	represent pointers.
+
+	* kaffe/kaffevm/thread.c
+	(nameThread): Check that the thread has been linked before returning
+	the name.
+
+	* kaffe/kaffevm/kaffe-gc/gc-mem.c
+	(page_alloc): Use intp instead of intptr_t as it is not available on
+	all systems.
+	
+	* kaffe/kaffevm/kaffe-gc/gc-refs.c
+	(TwalkLiveThreads): Reuse jthread_walkLiveThreads_r to highlight the
+	bug on SMP.
+
+	* kaffe/kaffevm/systems/unix-jthreads/jsignal.h: Removed ignoreSignal.
+
+	* kaffe/kaffevm/systems/unix-pthreads/syscalls.c
+	(jthreadedForkExec): Save the old sigmask and restore it after fork.
+
+	* kaffe/kaffevm/systems/unix-pthreads/thread-impl.c
+	(jthread_create): Cleanup the thread queue if pthread_create fails.
+	(jthread_suspendall): Temporarily changed logic to insure that all
+	concerned threads receive the signal.
+
+2004-12-29  Guilhem Lavaux  <guilhem at kaffe.org>
+
 	* config/sparc/jit-sparc.def: Renamed setEpilogueLabel to match the
 	new symbol names.
 
Index: kaffe/kaffe/kaffevm/gtypes.h
diff -u kaffe/kaffe/kaffevm/gtypes.h:1.14 kaffe/kaffe/kaffevm/gtypes.h:1.15
--- kaffe/kaffe/kaffevm/gtypes.h:1.14	Wed Dec  1 02:41:51 2004
+++ kaffe/kaffe/kaffevm/gtypes.h	Wed Dec 29 16:27:42 2004
@@ -94,8 +94,10 @@
 
 #if SIZEOF_VOID_P == 4
 typedef uint32			uintp;
+typedef int32			intp;
 #elif SIZEOF_VOID_P == 8
 typedef uint64			uintp;
+typedef int64			intp;
 #else
 #error "sizeof(void*) must be 4 or 8"
 #endif
Index: kaffe/kaffe/kaffevm/thread.c
diff -u kaffe/kaffe/kaffevm/thread.c:1.89 kaffe/kaffe/kaffevm/thread.c:1.90
--- kaffe/kaffe/kaffevm/thread.c:1.89	Sat Dec 25 19:09:13 2004
+++ kaffe/kaffe/kaffevm/thread.c	Wed Dec 29 16:27:42 2004
@@ -538,7 +538,10 @@
 {
 	static char buf[80];
 
-	stringJava2CBuf(unhand(unhand(tid)->thread)->name, buf, sizeof(buf));
+	if (tid != NULL && unhand(tid)->thread != NULL && unhand(unhand(tid)->thread)->name != NULL)
+	  stringJava2CBuf(unhand(unhand(tid)->thread)->name, buf, sizeof(buf));
+	else
+	  strcpy(buf, "<null name>");
 
 	return buf;
 }
Index: kaffe/kaffe/kaffevm/kaffe-gc/gc-mem.c
diff -u kaffe/kaffe/kaffevm/kaffe-gc/gc-mem.c:1.22 kaffe/kaffe/kaffevm/kaffe-gc/gc-mem.c:1.23
--- kaffe/kaffe/kaffevm/kaffe-gc/gc-mem.c:1.22	Tue Dec 21 05:50:02 2004
+++ kaffe/kaffe/kaffevm/kaffe-gc/gc-mem.c	Wed Dec 29 16:27:42 2004
@@ -980,7 +980,7 @@
 	 */
 	for (;;) {
 		int missed;
-		ptr = sbrk((intptr_t)size);
+		ptr = sbrk((intp)size);
 		if (ptr == (void*)-1) {
 			ptr = NULL;
 			break;
@@ -992,7 +992,7 @@
 		DBG(GCSYSALLOC,
 		    dprintf("unaligned sbrk %p, missed %d bytes\n",
 			    ptr, missed));
-		sbrk((intptr_t)(-size + missed));
+		sbrk((intp)(-size + missed));
 	}
 	CHECK_OUT_OF_MEMORY(ptr);
 
@@ -1028,7 +1028,7 @@
 #ifdef HAVE_SBRK
 static void pagefree(uintp base UNUSED, size_t size)
 {
-	sbrk((intptr_t)-size);
+	sbrk((intp)-size);
 }
 #else
 static void pagefree(uintp base, size_t size UNUSED)
Index: kaffe/kaffe/kaffevm/kaffe-gc/gc-refs.c
diff -u kaffe/kaffe/kaffevm/kaffe-gc/gc-refs.c:1.9 kaffe/kaffe/kaffevm/kaffe-gc/gc-refs.c:1.10
--- kaffe/kaffe/kaffevm/kaffe-gc/gc-refs.c:1.9	Sat Dec 25 19:09:16 2004
+++ kaffe/kaffe/kaffevm/kaffe-gc/gc-refs.c	Wed Dec 29 16:27:42 2004
@@ -297,7 +297,7 @@
   * registered.  Terminating a thread will remove it from the
   * threading system, and then we won't walk it here anymore
   */
- KTHREAD(walkLiveThreads)(liveThreadWalker, collector);
+ KTHREAD(walkLiveThreads_r)(liveThreadWalker, collector);
  DBG(GCWALK,
      dprintf("Following references now...\n");
      );
Index: kaffe/kaffe/kaffevm/systems/unix-jthreads/jsignal.h
diff -u kaffe/kaffe/kaffevm/systems/unix-jthreads/jsignal.h:1.5 kaffe/kaffe/kaffevm/systems/unix-jthreads/jsignal.h:1.6
--- kaffe/kaffe/kaffevm/systems/unix-jthreads/jsignal.h:1.5	Sun May  9 14:20:12 2004
+++ kaffe/kaffe/kaffevm/systems/unix-jthreads/jsignal.h	Wed Dec 29 16:27:42 2004
@@ -39,7 +39,6 @@
 
 void registerTerminalSignal(int sig, void* handler);
 
-void ignoreSignal(int sig);
 void clearSignal(int sig);
 void unblockSignal(int sig);
 void unblockAsyncSignals(void);
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/jsignal.h
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/jsignal.h:1.4 kaffe/kaffe/kaffevm/systems/unix-pthreads/jsignal.h:1.5
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/jsignal.h:1.4	Wed Dec 29 11:28:33 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/jsignal.h	Wed Dec 29 16:27:43 2004
@@ -37,10 +37,11 @@
 
 void registerTerminalSignal(int sig, void* handler);
 
-void clearSignal(int sig);
 void unblockSignal(int sig);
+void clearSignal(int sig);
 void unblockAsyncSignals(void);
 void blockAsyncSignals(void);
+void unblockAndCheckSignal(int sig);
 
 void detectStackBoundaries(jthread_t jtid, int mainThreadStackSize);
 
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/signal.c
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/signal.c:1.16 kaffe/kaffe/kaffevm/systems/unix-pthreads/signal.c:1.17
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/signal.c:1.16	Wed Dec 29 11:28:33 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/signal.c	Wed Dec 29 16:27:43 2004
@@ -311,7 +311,6 @@
 	sigprocmask(SIG_UNBLOCK, &nsig, NULL);
 }
 
-
 /*
  * Unblock all of the async signals.  (These are the
  * signals specified in the sa_mask in registerAsynchSignalHandler().)
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/syscalls.c
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/syscalls.c:1.25 kaffe/kaffe/kaffevm/systems/unix-pthreads/syscalls.c:1.26
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/syscalls.c:1.25	Sun Dec 26 05:52:33 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/syscalls.c	Wed Dec 29 16:27:43 2004
@@ -663,7 +663,7 @@
 
 	int fds[8];
 	int nfd;		/* number of fds in `fds' that are valid */
-	sigset_t nsig;
+	sigset_t nsig, osig;
 	char b[1];
 	int pid, i, err;
 
@@ -696,7 +696,7 @@
 	 * reenable signals in the child after we cleaned up.
 	 */
 	sigfillset(&nsig);
-	sigprocmask(SIG_BLOCK, &nsig, NULL);
+	sigprocmask(SIG_BLOCK, &nsig, &osig);
 
 	pid = fork();
 
@@ -752,7 +752,7 @@
 		err = errno;
 		/* Close all pipe fds */
 		close_fds(fds, 8);
-		sigprocmask(SIG_UNBLOCK, &nsig, NULL);
+		sigprocmask(SIG_UNBLOCK, &osig, NULL);
 		return (err);
 
 	default:
@@ -769,7 +769,7 @@
 		ioes[2] = fds[ERR_IN];
 		ioes[3] = fds[SYNC_OUT];
 
-		sigprocmask(SIG_UNBLOCK, &nsig, NULL);
+		sigprocmask(SIG_UNBLOCK, &osig, NULL);
 		*outpid = pid;
 		return (0);
 	}
Index: kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c
diff -u kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.63 kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.64
--- kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c:1.63	Sat Dec 25 19:09:17 2004
+++ kaffe/kaffe/kaffevm/systems/unix-pthreads/thread-impl.c	Wed Dec 29 16:27:43 2004
@@ -915,7 +915,6 @@
 	 * the newly created thread is set up correctly (i.e. is walkable)
 	 */
 	protectThreadList(cur);
-
 	nt->active = 1;
 	nt->next = activeThreads;
 	activeThreads = nt;
@@ -945,15 +944,17 @@
 	  }
 
 	  repsem_destroy( &nt->sem);
-	  unprotectThreadList(cur);
 	  KGC_rmRef(threadCollector, nt);
+	  nt->active = 0;
+	  activeThreads = nt->next;
+	  unprotectThreadList(cur);
 	  return NULL;
 	}
-
 	/* wait until the thread specific data has been set, and the new thread
 	 * is in a suspendable state */
 	repsem_wait( &nt->sem);
 
+	/* The key is installed. We can now let the signals coming. */
 	unprotectThreadList(cur);
   }
   return (nt);
@@ -1260,6 +1261,12 @@
   if ( ++critSection == 1 ){
 
 #if !defined(KAFFE_BOEHM_GC)
+	int val;
+	int numPending = 0;
+
+	repsem_getvalue(&critSem, &val);
+	assert(val == 0);
+
 	for ( t=activeThreads; t; t = t->next ){
 	  /*
 	   * make sure we don't suspend ourselves, and we don't expect suspend
@@ -1284,7 +1291,8 @@
 		  {
 		    if ((status = pthread_kill( t->tid, sigSuspend)) != 0)
 		      {
-			DBG( JTHREAD, dprintf("error sending SUSPEND signal to %p: %d\n", t, status));
+			dprintf("Internal error: error sending SUSPEND signal to %p: %d (%s)\n", t, status, strerror(status));
+			ABORT();
 		      }
 		    else
 		      {
@@ -1292,13 +1300,32 @@
 			 * It shouldn't be necessary (posix sems are accumulative), and it
 			 * is bad, performancewise (at least n*2 context switches per suspend)
 			 * but it works more reliably on linux 2.2.x */
-			repsem_wait( &critSem);
+			numPending++;
 		      }
 		  }
 	  }
 	  pthread_mutex_unlock(&t->suspendLock);
 	}
 
+	/* Now that all signals has been sent we may wait for all concerned
+	 * threads to handle them.
+	 */
+	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);
+	  }
+
 #else
 	/*
 	 * Here, we must use the special Boehm's stop world routine.
@@ -1339,6 +1366,7 @@
 	 * we cannot use mutexes as they cause a deadlock when the world
 	 * is suspended.
 	 */
+	  protectThreadList(cur);
 
 #if !defined(KAFFE_BOEHM_GC)
 	for ( t=activeThreads; t; t = t->next ){
@@ -1380,6 +1408,7 @@
 	GC_start_world();
 
 #endif
+	  unprotectThreadList(cur);
 
   }
 




More information about the kaffe mailing list