From a9ccc34071513a13b439eaadebfd3c05dd940392 Mon Sep 17 00:00:00 2001 From: Daniel Barlow Date: Mon, 6 Oct 2003 16:48:38 +0000 Subject: [PATCH] 0.8.4.8 More thread fixes: now passes the cl-ppcre thread test ... all_threads_lock needed to be declared volatile ... new 'state' field in struct thread, to avoid race where the head of the all_threads list is reaped during stop_for_gc Removed "waitpid : child %d %x exited \n" message (rather, conditionalised it on show_thread_exit, which is default 0 Remove unexplained (setsigmask 0) call in the repl, replaced with a call to warn_when_signals_masked so we can find out what it was for anyway. Default repl calls (get-foreground) - this is a unithread nop --- src/code/target-signal.lisp | 12 ++++++++---- src/code/toplevel.lisp | 3 ++- src/compiler/generic/objdef.lisp | 1 + src/runtime/interrupt.c | 29 ++++++++++++++++++++++++++++- src/runtime/runtime.c | 4 +++- src/runtime/thread.c | 21 ++++++++++++++++++--- src/runtime/thread.h | 4 ++++ version.lisp-expr | 2 +- 8 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/code/target-signal.lisp b/src/code/target-signal.lisp index 58121c1..f5a346e 100644 --- a/src/code/target-signal.lisp +++ b/src/code/target-signal.lisp @@ -23,8 +23,7 @@ (signal sb!alien:int)) ;;; Send the signal SIGNAL to the process with process id PID. SIGNAL -;;; should be a valid signal number or a keyword of the standard UNIX -;;; signal name. +;;; should be a valid signal number (defun unix-kill (pid signal) (real-unix-kill pid signal)) @@ -34,8 +33,7 @@ (signal sb!alien:int)) ;;; Send the signal SIGNAL to the all the process in process group -;;; PGRP. SIGNAL should be a valid signal number or a keyword of the -;;; standard UNIX signal name. +;;; PGRP. SIGNAL should be a valid signal number (defun unix-killpg (pgrp signal) (real-unix-killpg pgrp signal)) @@ -57,7 +55,13 @@ sb!alien:unsigned-long (signal sb!alien:int) (handler sb!alien:unsigned-long)) +;;; assert (though non-fatally) that there are no signals masked +(sb!alien:define-alien-routine "warn_when_signals_masked" sb!alien:void) + + + + ;;;; interface to enabling and disabling signal handlers (defun enable-interrupt (signal handler) diff --git a/src/code/toplevel.lisp b/src/code/toplevel.lisp index bbfd22b..2cc0062 100644 --- a/src/code/toplevel.lisp +++ b/src/code/toplevel.lisp @@ -510,7 +510,7 @@ (abort "~@") (catch 'toplevel-catcher - #!-sunos (sb!unix:unix-sigsetmask 0) ; FIXME: What is this for? + (sb!unix::warn-when-signals-masked) ;; in the event of a control-stack-exhausted-error, we should ;; have unwound enough stack by the time we get here that this ;; is now possible @@ -553,6 +553,7 @@ (loop ;; (See comment preceding the definition of SCRUB-CONTROL-STACK.) (scrub-control-stack) + (sb!thread::get-foreground) (unless noprint (funcall *repl-prompt-fun* *standard-output*) ;; (Should *REPL-PROMPT-FUN* be responsible for doing its own diff --git a/src/compiler/generic/objdef.lisp b/src/compiler/generic/objdef.lisp index 16f8a6b..590d146 100644 --- a/src/compiler/generic/objdef.lisp +++ b/src/compiler/generic/objdef.lisp @@ -384,6 +384,7 @@ (tls-cookie) ; on x86, the LDT index (this :c-type "struct thread *" :length #!+alpha 2 #!-alpha 1) (next :c-type "struct thread *" :length #!+alpha 2 #!-alpha 1) + (state) ; running, stopping, stopped #!+x86 (pseudo-atomic-atomic) #!+x86 (pseudo-atomic-interrupted) (interrupt-data :c-type "struct interrupt_data *" diff --git a/src/runtime/interrupt.c b/src/runtime/interrupt.c index 3250875..dac50f7 100644 --- a/src/runtime/interrupt.c +++ b/src/runtime/interrupt.c @@ -69,7 +69,7 @@ static void store_signal_data_for_later (struct interrupt_data *data, os_context_t *context); boolean interrupt_maybe_gc_int(int signal, siginfo_t *info, void *v_context); -extern lispobj all_threads_lock; +extern volatile lispobj all_threads_lock; extern volatile int countdown_to_gc; /* @@ -117,6 +117,32 @@ boolean internal_errors_enabled = 0; struct interrupt_data * global_interrupt_data; +/* this is used from Lisp in toplevel.lisp, replacing an older + * (sigsetmask 0) - we'd like to find out when the signal mask is + * not 0 */ + +/* This check was introduced in 0.8.4.x and some day will go away + * again unless we find a way to trigger it */ + +void warn_when_signals_masked () +{ + /* and as a side-eeffect, unmask them */ + sigset_t new,old; + int i; + int wrong=0; + sigemptyset(&new); + sigprocmask(SIG_SETMASK,&new,&old); + for(i=1; ipid); countdown_to_gc--; + thread->state=STATE_STOPPED; release_spinlock(&all_threads_lock); kill(thread->pid,SIGSTOP); diff --git a/src/runtime/runtime.c b/src/runtime/runtime.c index ae360ae..d7e672c 100644 --- a/src/runtime/runtime.c +++ b/src/runtime/runtime.c @@ -379,6 +379,7 @@ static void parent_sighandler(int signum,siginfo_t *info, void *void_context) } #ifdef LISP_FEATURE_SB_THREAD +int show_thread_exit=0; static void /* noreturn */ parent_loop(void) { @@ -421,7 +422,8 @@ static void /* noreturn */ parent_loop(void) if(WIFEXITED(status) || WIFSIGNALED(status)) { th=find_thread_by_pid(pid); if(!th) continue; - fprintf(stderr,"waitpid : child %d %x exited \n", pid,th); + if(show_thread_exit) + fprintf(stderr,"waitpid : child %d %x exited \n", pid,th); destroy_thread(th); if(!all_threads) break; } diff --git a/src/runtime/thread.c b/src/runtime/thread.c index a42c26e..8bd4488 100644 --- a/src/runtime/thread.c +++ b/src/runtime/thread.c @@ -24,7 +24,7 @@ int dynamic_values_bytes=4096*sizeof(lispobj); /* same for all threads */ struct thread *all_threads; -lispobj all_threads_lock; +volatile lispobj all_threads_lock; volatile int countdown_to_gc; extern struct interrupt_data * global_interrupt_data; @@ -238,6 +238,7 @@ void destroy_thread (struct thread *th) #endif get_spinlock(&all_threads_lock,th->pid); if(countdown_to_gc>0) countdown_to_gc--; + th->state=STATE_STOPPED; if(th==all_threads) all_threads=th->next; else { @@ -321,8 +322,21 @@ void gc_stop_the_world() * for them on each time around */ for(p=all_threads;p!=tail;p=p->next) { if(p==th) continue; - countdown_to_gc++; - kill(p->pid,SIG_STOP_FOR_GC); + /* if the head of all_threads is removed during + * gc_stop_the_world, we may take a second trip through the + * list and end up counting twice as many threads to wait for + * as actually exist */ + if(p->state!=STATE_RUNNING) continue; + countdown_to_gc++; + p->state=STATE_STOPPING; + /* Note no return value check from kill(). If the + * thread had been reaped already, we kill it and + * increment countdown_to_gc anyway. This is to avoid + * complicating the logic in destroy_thread, which would + * otherwise have to know whether the thread died before or + * after it was killed + */ + kill(p->pid,SIG_STOP_FOR_GC); } tail=all_threads; } else { @@ -339,6 +353,7 @@ void gc_start_the_world() get_spinlock(&all_threads_lock,th->pid); for(p=all_threads;p;p=p->next) { if(p==th) continue; + p->state=STATE_RUNNING; kill(p->pid,SIGCONT); } release_spinlock(&all_threads_lock); diff --git a/src/runtime/thread.h b/src/runtime/thread.h index 9fb3ab3..e9039a1 100644 --- a/src/runtime/thread.h +++ b/src/runtime/thread.h @@ -18,6 +18,10 @@ struct alloc_region { }; #include "genesis/static-symbols.h" #include "genesis/thread.h" +#define STATE_RUNNING (make_fixnum(0)) +#define STATE_STOPPING (make_fixnum(1)) +#define STATE_STOPPED (make_fixnum(2)) + #define THREAD_SLOT_OFFSET_WORDS(c) \ (offsetof(struct thread,c)/(sizeof (struct thread *))) diff --git a/version.lisp-expr b/version.lisp-expr index 95b6ccb..8e856ab 100644 --- a/version.lisp-expr +++ b/version.lisp-expr @@ -17,4 +17,4 @@ ;;; checkins which aren't released. (And occasionally for internal ;;; versions, especially for internal versions off the main CVS ;;; branch, it gets hairier, e.g. "0.pre7.14.flaky4.13".) -"0.8.4.7" +"0.8.4.8" -- 1.7.10.4