From 5de51c1ffd9b140b88113137aa22e9d59a01f4f5 Mon Sep 17 00:00:00 2001 From: Douglas Katzman Date: Tue, 2 Mar 2021 23:28:04 -0500 Subject: [PATCH] Remove :SB-THRUPTION feature keyword Everything it guarded is changed to test :SB-SAFEPOINT instead. --- base-target-features.lisp-expr | 10 +--------- crossbuild-runner/build-all.sh | 2 +- make-config.sh | 2 +- package-data-list.lisp-expr | 2 +- src/code/early-impl.lisp | 2 +- src/code/signal.lisp | 6 +++--- src/code/target-signal-common.lisp | 4 ++-- src/code/target-signal.lisp | 4 ++-- src/code/target-thread.lisp | 6 +++--- src/code/toplevel.lisp | 4 ++-- src/cold/shared.lisp | 2 -- src/compiler/generic/parms.lisp | 4 ++-- src/runtime/interrupt.c | 8 ++++---- src/runtime/interrupt.h | 2 +- src/runtime/safepoint.c | 14 +++++++------- src/runtime/thread.c | 4 ++-- src/runtime/thread.h | 2 +- tests/kill-non-lisp-thread.impure.lisp | 4 ++-- tests/test-util.lisp | 8 +------- tests/timer.impure.lisp | 4 ++-- 20 files changed, 39 insertions(+), 55 deletions(-) diff --git a/base-target-features.lisp-expr b/base-target-features.lisp-expr index 48f8456b3..f1c6d7999 100644 --- a/base-target-features.lisp-expr +++ b/base-target-features.lisp-expr @@ -290,15 +290,7 @@ ;; (Replaces use of SIG_STOP_FOR_GC.) ; :sb-safepoint - ;; When compiling with safepoints, the INTERRUPT-THREAD mechanism can - ;; also use safepoints to roll the target thread to a point at which it - ;; can be interrupted safely, instead of using a signal for this - ;; purpose. Enable this feature in addition to :SB-SAFEPOINT to enable - ;; such behaviour. - ;; (Replaces use of SIGURG, except to wake up syscalls.) - ; :sb-thruption - - ;; When compiling with safepoints and thruptions, the TIMER facility + ;; When compiling with safepoints, the TIMER facility ;; can replace its use of setitimer with a background thread. ;; (Replaces use of SIGALRM.) ; :sb-wtimer diff --git a/crossbuild-runner/build-all.sh b/crossbuild-runner/build-all.sh index ee647077e..5f734da76 100755 --- a/crossbuild-runner/build-all.sh +++ b/crossbuild-runner/build-all.sh @@ -26,7 +26,7 @@ do echo ":$arch" >> $ltf # x86 and x86-64 are tested as if #+win32. Unix is otherwise plenty tested. if [ $arch = x86 -o $arch = x86-64 ]; then - echo ':win32 :sb-thread :sb-safepoint :sb-thruption :sb-wtimer' >> $ltf + echo ':win32 :sb-thread :sb-safepoint :sb-wtimer' >> $ltf else echo ':unix :linux :elf' >> $ltf fi diff --git a/make-config.sh b/make-config.sh index b40ddb930..bdf29be8e 100755 --- a/make-config.sh +++ b/make-config.sh @@ -592,7 +592,7 @@ case "$sbcl_os" in # (Of course it doesn't provide dlopen, but there is # roughly-equivalent magic nevertheless:) printf ' :os-provides-dlopen' >> $ltf - printf ' :sb-thread :sb-safepoint :sb-thruption :sb-wtimer' >> $ltf + printf ' :sb-thread :sb-safepoint :sb-wtimer' >> $ltf # link_or_copy Config.$sbcl_arch-win32 Config link_or_copy $sbcl_arch-win32-os.h target-arch-os.h diff --git a/package-data-list.lisp-expr b/package-data-list.lisp-expr index d44f605a1..bb2fbc5ce 100644 --- a/package-data-list.lisp-expr +++ b/package-data-list.lisp-expr @@ -2771,7 +2771,7 @@ SB-KERNEL) have been undone, but probably more remain." "*ALLOW-WITH-INTERRUPTS*" "*INTERRUPTS-ENABLED*" "*INTERRUPT-PENDING*" - #+sb-thruption "*THRUPTION-PENDING*" + #+sb-safepoint "*THRUPTION-PENDING*" "*LINKAGE-INFO*" "*LONG-SITE-NAME*" "*SHORT-SITE-NAME*" "*MACHINE-VERSION*" diff --git a/src/code/early-impl.lisp b/src/code/early-impl.lisp index 14b0e4706..6e7030436 100644 --- a/src/code/early-impl.lisp +++ b/src/code/early-impl.lisp @@ -27,7 +27,7 @@ sb-unix::*unblock-deferrables-on-enabling-interrupts-p* *interrupts-enabled* *interrupt-pending* - #+sb-thruption *thruption-pending* + #+sb-safepoint *thruption-pending* #+sb-safepoint *in-safepoint* *free-interrupt-context-index* #-gencgc diff --git a/src/code/signal.lisp b/src/code/signal.lisp index dbde366c3..110228404 100644 --- a/src/code/signal.lisp +++ b/src/code/signal.lisp @@ -51,7 +51,7 @@ ;;; These 4 symbols are initialized by create_thread_struct() (defvar *interrupts-enabled*) (defvar *interrupt-pending*) -#+sb-thruption (defvar *thruption-pending*) +#+sb-safepoint (defvar *thruption-pending*) (defvar *allow-with-interrupts*) ;;; This is to support signal handlers that want to return to the @@ -71,7 +71,7 @@ (dolist (symbol '(*unblock-deferrables-on-enabling-interrupts-p* *interrupts-enabled* *interrupt-pending* - #+sb-thruption *thruption-pending* + #+sb-safepoint *thruption-pending* *allow-with-interrupts*)) ;; Force these to be always bound despite absence of a compile-time binding. ;; (Avoid accidentally installing a value into symbol->value in cold-load) @@ -154,7 +154,7 @@ WITHOUT-INTERRUPTS in: ;; handled immediately upon exit from said ;; WITHOUT-INTERRUPTS, so it is as if nothing has happened. (when (or *interrupt-pending* - #+sb-thruption *thruption-pending*) + #+sb-safepoint *thruption-pending*) (receive-pending-interrupt))) (,without-interrupts-body))))) diff --git a/src/code/target-signal-common.lisp b/src/code/target-signal-common.lisp index 291f63468..833380f08 100644 --- a/src/code/target-signal-common.lisp +++ b/src/code/target-signal-common.lisp @@ -41,7 +41,7 @@ (let (*unblock-deferrables-on-enabling-interrupts-p*) (unblock-deferrable-signals) (when (or *interrupt-pending* - #+sb-thruption *thruption-pending*) + #+sb-safepoint *thruption-pending*) (receive-pending-interrupt)) (funcall function)) (alien-funcall (extern-alien "block_deferrable_signals" @@ -50,7 +50,7 @@ (t (when (and enable-interrupts (or *interrupt-pending* - #+sb-thruption *thruption-pending*)) + #+sb-safepoint *thruption-pending*)) (receive-pending-interrupt)) (funcall function)))) diff --git a/src/code/target-signal.lisp b/src/code/target-signal.lisp index 6df208855..0c71d34fb 100644 --- a/src/code/target-signal.lisp +++ b/src/code/target-signal.lisp @@ -170,7 +170,7 @@ (declare (ignore signal code context)) (exit)) -#-sb-thruption +#-sb-safepoint ;;; SIGURG is not used in SBCL for its original purpose, instead it's ;;; for signalling a thread that it should look at its interruption ;;; queue. The handler (RUN_INTERRUPTION) just returns if there is @@ -212,7 +212,7 @@ " sb-sys:*stderr*)) #-(or linux android) (%install-handler sigsys #'sigsys-handler) #-sb-wtimer (%install-handler sigalrm #'sigalrm-handler) - #-sb-thruption (%install-handler sigurg #'sigurg-handler) + #-sb-safepoint (%install-handler sigurg #'sigurg-handler) (%install-handler sigchld #'sigchld-handler) ;; Don't want to silently quit on broken pipes. (%install-handler sigpipe :ignore) diff --git a/src/code/target-thread.lisp b/src/code/target-thread.lisp index a3a1760cc..eb4682982 100644 --- a/src/code/target-thread.lisp +++ b/src/code/target-thread.lisp @@ -1849,7 +1849,7 @@ session." ;; interupts to be lost: sigint comes to ;; mind. (setq *interrupt-pending* nil) - #+sb-thruption + #+sb-safepoint (setq *thruption-pending* nil) (handle-thread-exit))))))) ;; this returns to C, so return a single value @@ -2120,7 +2120,7 @@ subject to change." (function destroy-thread :replacement terminate-thread))) ;;; Called from the signal handler. -#-(or sb-thruption win32) +#-(or sb-safepoint win32) (defun run-interruption () (let ((interruption (with-deathlok (*current-thread*) (pop (thread-interruptions *current-thread*))))) @@ -2135,7 +2135,7 @@ subject to change." (when interruption (funcall interruption)))) -#+sb-thruption +#+sb-safepoint (defun run-interruption (*current-internal-error-context*) (in-interruption () ;the non-thruption code does this in the signal handler (let ((interruption (with-deathlok (*current-thread*) diff --git a/src/code/toplevel.lisp b/src/code/toplevel.lisp index 125ae9b86..0deea484c 100644 --- a/src/code/toplevel.lisp +++ b/src/code/toplevel.lisp @@ -132,11 +132,11 @@ means to wait indefinitely.") ;;; cancelation points (system and C library calls) which check for cancelation, ;;; perform whatever cleanups were pushed by pthread_cleanup_push() and then stop. ;;; -;;; P.S. To see that #+sb-thruption is no magic fix - suppose you have a stack +;;; P.S. To see that #+sb-safepoint is no magic fix - suppose you have a stack ;;; with Lisp -> C -> -> Lisp where C acquired a resource and the currently ;;; top-of-stack Lisp function took a safepoint trap. It would be fine if all it ;;; wanted to do was GC, but is not safe in general. -;;; Hence #+sb-thruption paints a dangerously attractive veneer over an unsafe +;;; Hence #+sb-safepoint paints a dangerously attractive veneer over an unsafe ;;; concept, making it more subtly bad instead of very obviously bad. ;;; ;;; So this is a bad default. Bad bad bad. But it's backward-compatible. diff --git a/src/cold/shared.lisp b/src/cold/shared.lisp index c1a844bd3..dd6ac7c4f 100644 --- a/src/cold/shared.lisp +++ b/src/cold/shared.lisp @@ -300,8 +300,6 @@ ("(and pauseless-threadstart (not sb-thread))" ":PAUSELESS-THREADSTART requires :SB-THREAD") ("(and sb-safepoint (not sb-thread))" ":SB-SAFEPOINT requires :SB-THREAD") - ("(and sb-thruption (not sb-safepoint))" ":SB-THRUPTION requires :SB-SAFEPOINT") - ("(and (not sb-thruption) sb-safepoint)" ":SB-SAFEPOINT requires :SB-THRUPTION") ("(and sb-thread (not (or riscv ppc ppc64 x86 x86-64 arm64)))" ":SB-THREAD not supported on selected architecture") ("(and gencgc cheneygc)" diff --git a/src/compiler/generic/parms.lisp b/src/compiler/generic/parms.lisp index 4f0d9bfe3..1c80591af 100644 --- a/src/compiler/generic/parms.lisp +++ b/src/compiler/generic/parms.lisp @@ -147,7 +147,7 @@ sb-di::handle-breakpoint sb-di::handle-single-step-trap #+win32 sb-kernel::handle-win32-exception - #+sb-thruption sb-thread::run-interruption + #+sb-safepoint sb-thread::run-interruption enter-alien-callback #+sb-thread sb-thread::enter-foreign-callback) #'equal) @@ -164,7 +164,7 @@ (sb-sys:*allow-with-interrupts* t) (sb-sys:*interrupts-enabled* t) sb-sys:*interrupt-pending* - #+sb-thruption sb-sys:*thruption-pending* + #+sb-safepoint sb-sys:*thruption-pending* *in-without-gcing* *gc-inhibit* *gc-pending* diff --git a/src/runtime/interrupt.c b/src/runtime/interrupt.c index 6ca585b37..f5190851d 100644 --- a/src/runtime/interrupt.c +++ b/src/runtime/interrupt.c @@ -1171,7 +1171,7 @@ interrupt_handle_pending(os_context_t *context) sigcopyset(os_context_sigmask_addr(context), &data->pending_mask); run_deferred_handler(data, context); } -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT if (read_TLS(THRUPTION_PENDING,thread)==T) /* Special case for the following situation: There is a * thruption pending, but a signal had been deferred. The @@ -1863,8 +1863,8 @@ low_level_handle_now_handler(int signal, siginfo_t *info, void *void_context) * As well there are two asynchronous signals installed via this function: * - STOP_FOR_GC is low-level, but might defer the signal through * an intricate bunch of decisions about the state of the world. - * - SIGURG without sb-thruption is a high-level (Lisp) handler, - * but with sb-thruption is low-level handler that uses different + * - SIGURG without :SB-SAFEPOINT is a high-level (Lisp) handler, + * but with :SB-SAFEPOINT is low-level handler that uses different * criteria for when to defer. */ void ll_install_handler (int signal, interrupt_handler_t handler) @@ -1872,7 +1872,7 @@ ll_install_handler (int signal, interrupt_handler_t handler) struct sigaction sa; if (0 > signal || signal >= NSIG -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT /* SIGURG is in `deferrable_sigset' so that we block&unblock it properly, * but we don't actually want to defer it, at least not here. * (It might get deferred until a safepoint). And if we put it only diff --git a/src/runtime/interrupt.h b/src/runtime/interrupt.h index 03e4b6fd6..528812fd6 100644 --- a/src/runtime/interrupt.h +++ b/src/runtime/interrupt.h @@ -137,7 +137,7 @@ extern void lower_thread_control_stack_guard_page(struct thread *th); extern void reset_thread_control_stack_guard_page(struct thread *th); #if defined(LISP_FEATURE_SB_SAFEPOINT) && !defined(LISP_FEATURE_WIN32) -# ifdef LISP_FEATURE_SB_THRUPTION +# ifdef LISP_FEATURE_SB_SAFEPOINT void thruption_handler(int signal, siginfo_t *info, os_context_t *context); # endif #endif diff --git a/src/runtime/safepoint.c b/src/runtime/safepoint.c index 5af5a6541..3f874156d 100644 --- a/src/runtime/safepoint.c +++ b/src/runtime/safepoint.c @@ -484,7 +484,7 @@ thread_register_gc_trigger() } } -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT static inline int thread_may_thrupt(os_context_t *ctx) { @@ -740,7 +740,7 @@ void thread_in_lisp_raised(os_context_t *ctxptr) * SUB-GC. Phase is either GC_QUIET or GC_NONE. */ if (check_gc_and_thruptions) { check_pending_gc(ctxptr); -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT while(check_pending_thruptions(ctxptr)); #endif } @@ -798,7 +798,7 @@ void thread_in_safety_transition(os_context_t *ctxptr) } } } -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT if (was_in_alien) { while(check_pending_thruptions(ctxptr)); } @@ -826,7 +826,7 @@ void thread_interrupted(os_context_t *ctxptr) } } check_pending_gc(ctxptr); -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT while(check_pending_thruptions(ctxptr)); #endif } @@ -896,7 +896,7 @@ void gc_start_the_world() } -#ifdef LISP_FEATURE_SB_THRUPTION +#ifdef LISP_FEATURE_SB_SAFEPOINT /* wake_thread(thread) -- ensure a thruption delivery to * `thread'. */ @@ -997,7 +997,7 @@ void wake_thread_impl(struct thread_instance *lispthread) thread_sigmask(SIG_SETMASK, &oldset, 0); } #endif /* !LISP_FEATURE_WIN32 */ -#endif /* LISP_FEATURE_SB_THRUPTION */ +#endif /* LISP_FEATURE_SB_SAFEPOINT */ void* os_get_csp(struct thread* th) { @@ -1012,7 +1012,7 @@ void* os_get_csp(struct thread* th) #ifndef LISP_FEATURE_WIN32 -# ifdef LISP_FEATURE_SB_THRUPTION +# ifdef LISP_FEATURE_SB_SAFEPOINT /* This is basically what 'low_level_maybe_now_maybe_later' was (which doesn't exist), * but with a different name, and different way of deciding to defer the signal */ void thruption_handler(__attribute__((unused)) int signal, diff --git a/src/runtime/thread.c b/src/runtime/thread.c index 2f25908b1..46feed36c 100644 --- a/src/runtime/thread.c +++ b/src/runtime/thread.c @@ -1260,12 +1260,12 @@ void wake_thread(struct thread_instance* lispthread) block_deferrable_signals(&oldset); thread_mutex_lock(&all_threads_lock); sb_pthr_kill(thread, 1); // can't fail -# ifdef LISP_FEATURE_SB_THRUPTION +# ifdef LISP_FEATURE_SB_SAFEPOINT wake_thread_impl(lispthread); # endif thread_mutex_unlock(&all_threads_lock); thread_sigmask(SIG_SETMASK,&oldset,0); -#elif defined LISP_FEATURE_SB_THRUPTION +#elif defined LISP_FEATURE_SB_SAFEPOINT wake_thread_impl(lispthread); #else pthread_kill(lispthread->os_thread, SIGURG); diff --git a/src/runtime/thread.h b/src/runtime/thread.h index da6da2dfe..eb1099777 100644 --- a/src/runtime/thread.h +++ b/src/runtime/thread.h @@ -369,7 +369,7 @@ void thread_in_lisp_raised(os_context_t *ctx); void thread_interrupted(os_context_t *ctx); extern void thread_register_gc_trigger(); -# ifdef LISP_FEATURE_SB_THRUPTION +# ifdef LISP_FEATURE_SB_SAFEPOINT void wake_thread(struct thread_instance*), wake_thread_impl(struct thread_instance*); # endif diff --git a/tests/kill-non-lisp-thread.impure.lisp b/tests/kill-non-lisp-thread.impure.lisp index 2e75d7e11..c3ca816ce 100644 --- a/tests/kill-non-lisp-thread.impure.lisp +++ b/tests/kill-non-lisp-thread.impure.lisp @@ -44,8 +44,8 @@ (push (lambda () (setq receivedp t)) (sb-thread::thread-interruptions sb-thread:*current-thread*)) - #+sb-thruption - ;; On sb-thruption builds, the usual resignalling of SIGURG will + #+sb-safepoint + ;; On sb-safepoint builds, the usual resignalling of SIGURG will ;; work without problems, but the signal handler won't ordinarily ;; think that there's anything to be done. Since we're poking at ;; INTERRUPT-THREAD internals anyway, let's help it along. diff --git a/tests/test-util.lisp b/tests/test-util.lisp index 958158cec..92ac0782c 100644 --- a/tests/test-util.lisp +++ b/tests/test-util.lisp @@ -125,13 +125,7 @@ ;; needs to wait, to have a facsimile of the situation prior to implementation ;; of the so-called pauseless thread start feature. ;; Wouldn't you know, it's just reintroducing a startup semaphore. - ;; And interruption tests are even more likely to fail with sb-thruption - ;; because sb-thruption is flawed: it presumes that there is enough synchronization - ;; between sender/receiver that checking the INVOKED variable (shared via a closure) - ;; makes any sense at all, which it doesn't. (In addition, it supposes that merely - ;; by polling at safepoints, interrupts somehow become safe, which is not true - ;; in general - it is only true of the GC "interrupt" delivered by the kernel - ;; when the safepoint page trap is hit.) + ;; And interruption tests are even more likely to fail with :sb-safepoint. ;; Noneless, this tries to be robust enough to pass. (let* ((sem (sb-thread:make-semaphore)) (child (make-kill-thread diff --git a/tests/timer.impure.lisp b/tests/timer.impure.lisp index 818eb19a6..7a51d12ad 100644 --- a/tests/timer.impure.lisp +++ b/tests/timer.impure.lisp @@ -59,13 +59,13 @@ time) (loop until finishedp))) -;;; This test has to be skipped darwin + thruption because if those features +;;; This test has to be skipped on darwin + safepoint because if those features ;;; are present, then sb-wtimer should be too, but it can't be, because the ;;; code in 'darwin-os.c' says: ;;; # error Completely untested. Go ahead! Remove this line, try your luck! ;;; and of course it doesn't work. ;;; But win32 has wtimer so the skipped test is more than just -;;; (:and :sb-thruption (:not :sb-wtimer)) because that wouldn't +;;; (:and :sb-safepoint (:not :sb-wtimer)) because that wouldn't ;;; explain why win32 doesn't pass. (with-test (:name (:timer :deferrables-blocked) :skipped-on (or :win32 (:and :darwin :sb-safepoint)))