From: Daniel B. <da...@us...> - 2003-08-22 04:20:48
|
Update of /cvsroot/sbcl/sbcl/src/runtime In directory sc8-pr-cvs1:/tmp/cvs-serv27321/src/runtime Modified Files: Tag: stop_the_world_branch gencgc.c interrupt.c Log Message: 0.8.2.38.stop_the_world.6 Now we can interrupt-thread a thread that's GCing, and it won't ignore us. It will defer the interrupt until the GC is done, of course. Those changes in detail: (1) when deferring a GC from alloc(), we must remember that there is no signal handler context available, so signal mask should be taken from (and fiddled in) the normal process state (2) Don't put SIG_STOP_FOR_GC in the blockable set, or we can't GC threads that are waiting on queues (3) Remove #if 0 from around the copying of sigmask in undo_fake_foreign_function_call. Replace sizeof(sigmask_t) with an expression involving the value of NSIG and the rash assumption that sigset_t is a bitmask. (4) Commentary to indicate what's going on, and a couple of new tests which represent cases that yesterday's code couldn't cope with Index: gencgc.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/gencgc.c,v retrieving revision 1.35.2.4 retrieving revision 1.35.2.5 diff -u -d -r1.35.2.4 -r1.35.2.5 --- gencgc.c 21 Aug 2003 13:30:26 -0000 1.35.2.4 +++ gencgc.c 22 Aug 2003 01:24:48 -0000 1.35.2.5 @@ -4219,9 +4219,7 @@ /* set things up so that GC happens when we finish the PA * section. */ struct interrupt_data *data=th->interrupt_data; - os_context_t *context=get_interrupt_context_for_thread(th); - maybe_defer_handler - (interrupt_maybe_gc_int,data,0,0,context); + maybe_defer_handler(interrupt_maybe_gc_int,data,0,0,0); } new_obj = gc_alloc_with_region(nbytes,0,region,0); return (new_obj); Index: interrupt.c =================================================================== RCS file: /cvsroot/sbcl/sbcl/src/runtime/interrupt.c,v retrieving revision 1.41.2.3 retrieving revision 1.41.2.4 diff -u -d -r1.41.2.3 -r1.41.2.4 --- interrupt.c 20 Aug 2003 19:20:50 -0000 1.41.2.3 +++ interrupt.c 22 Aug 2003 01:24:48 -0000 1.41.2.4 @@ -70,6 +70,18 @@ extern lispobj all_threads_lock; extern int countdown_to_gc; +/* + * This is a workaround for some slightly silly Linux/GNU Libc + * behaviour: glibc defines sigset_t to support 1024 signals, which to + * is more than the kernel. This is usually not a problem, but + * becomes one when we want to save a signal mask from a ucontext, and + * restore it later into another ucontext: the ucontext is allocated + * on the stack by the kernel, so copying a libc-sized sigset_t into + * it will overflow and cause other data on the stack to be + * corrupted */ + +#define REAL_SIGSET_SIZE_BYTES ((NSIG/8)) + void sigaddset_blockable(sigset_t *s) { sigaddset(s, SIGHUP); @@ -90,7 +102,8 @@ sigaddset(s, SIGUSR1); sigaddset(s, SIGUSR2); #ifdef LISP_FEATURE_SB_THREAD - sigaddset(s, SIG_STOP_FOR_GC); + /* don't block STOP_FOR_GC, we need to be able to interrupt threads + * for GC purposes even when they are blocked on queues etc */ sigaddset(s, SIG_INTERRUPT_THREAD); #endif } @@ -201,6 +214,10 @@ foreign_function_call_active = 1; } +/* blocks all blockable signals. If you are calling from a signal handler, + * the usual signal mask will be restored from the context when the handler + * finishes. Otherwise, be careful */ + void undo_fake_foreign_function_call(os_context_t *context) { @@ -274,10 +291,15 @@ data=thread->interrupt_data; SetSymbolValue(INTERRUPT_PENDING, NIL,thread); -#if 0 - memcpy(os_context_sigmask_addr(context), &pending_mask, - 4 /* sizeof(sigset_t) */ ); -#endif + /* restore the saved signal mask from the original signal (the + * one that interrupted us during the critical section) into the + * os_context for the signal we're currently in the handler for. + * This should ensure that when we return from the handler the + * blocked signals are unblocked */ + + memcpy(os_context_sigmask_addr(context), &data->pending_mask, + REAL_SIGSET_SIZE_BYTES); + sigemptyset(&data->pending_mask); /* This will break on sparc linux: the deferred handler really wants * to be called with a void_context */ @@ -348,10 +370,13 @@ * be available; should we copy it or was nobody using it anyway?) * then we should convert this to return-elsewhere */ - /* Allocate the SAPs while the interrupts are still disabled. - * (FIXME: Why? This is the way it was done in CMU CL, and it - * even had the comment noting that this is the way it was - * done, but no motivation..) */ + /* CMUCL comment said "Allocate the SAPs while the interrupts + * are still disabled.". I (dan, 2003.08.21) assume this is + * because we're not in pseudoatomic and allocation shouldn't + * be interrupted. In which case it's no longer an issue as + * all our allocation from C now goes through a PA wrapper, + * but still, doesn't hurt */ + lispobj info_sap,context_sap = alloc_sap(context); info_sap = alloc_sap(info); /* Allow signals again. */ @@ -413,6 +438,9 @@ SetSymbolValue(INTERRUPT_PENDING, T,thread); return 1; } + /* a slightly confusing test. arch_pseudo_atomic_atomic() doesn't + * actually use its argument for anything on x86, so this branch + * may succeed even when context is null (gencgc alloc()) */ if ( #ifndef __i386__ (!foreign_function_call_active) && @@ -433,10 +461,25 @@ data->pending_signal = signal; if(info) memcpy(&(data->pending_info), info, sizeof(siginfo_t)); - memcpy(&(data->pending_mask), - os_context_sigmask_addr(context), - sizeof(sigset_t)); - sigaddset_blockable(os_context_sigmask_addr(context)); + if(context) { + /* the signal mask in the context (from before we were + * interrupted is copied to be restored when + * run_deferred_handler happens. Then the usually blocked + * signals are added to the context so that we are running + * with blocked signals when the handler returns */ + sigemptyset(&(data->pending_mask)); + memcpy(&(data->pending_mask), + os_context_sigmask_addr(context), + REAL_SIGSET_SIZE_BYTES); + sigaddset_blockable(os_context_sigmask_addr(context)); + } else { + /* this may be called from gencgc alloc(), in which case there + * has been no signal and therefore no context. */ + sigset_t new; + sigemptyset(&new); + sigaddset_blockable(&new); + sigprocmask(SIG_BLOCK,&new,&(data->pending_mask)); + } } |