Use a native mutex for already-in-gc

And consequently simplify ENTER-FOREIGN-CALLBACK
This commit is contained in:
Douglas Katzman 2020-09-07 02:17:05 -04:00
parent 83a5a0f1cd
commit 338c975db2
5 changed files with 29 additions and 52 deletions

View file

@ -292,23 +292,7 @@ the alien callback for that function with the given alien type."
(in-package "SB-THREAD")
#+sb-thread
(defun enter-foreign-callback (index return arguments)
(let ((thread
#+sb-safepoint ; cons a FOREIGN-THREAD
(without-gcing (init-thread-local-storage (make-foreign-thread)))
;; Deferrable signals are blocked. STOP_FOR_GC is not blocked but can't
;; occur because GC could only send it after acquiring the all_threads lock,
;; which it can't get because this thread owns it for the moment.
#-sb-safepoint ; use the preallocated FOREIGN-THREAD
(let ((thread (init-thread-local-storage *foreign-thread*)))
;; GC would be ok to happen now, but it can't yet because of the
;; all_threads_lock acting both as a guard on the preallocated thread
;; and the stop-the-world inhibitor.
;; There's no point in splitting that into two locks.
(setq *foreign-thread* (make-foreign-thread)) ; "Pay it forward"
(alien-funcall (extern-alien "release_all_threads_lock"
(function void)))
thread)))
(let ((thread (init-thread-local-storage (make-foreign-thread))))
#+pauseless-threadstart
(dx-let ((startup-info (vector nil ; trampoline is n/a
nil ; cell in *STARTING-THREADS* is n/a

View file

@ -147,7 +147,13 @@ statistics are appended to it."
;;; For GENCGC all generations < GEN will be GC'ed.
(define-load-time-global *already-in-gc* (sb-thread:make-mutex :name "GC lock"))
(defmacro try-acquire-gc-lock (&rest forms)
#-sb-thread `(progn ,@forms t)
#+sb-thread
`(when (eql (alien-funcall (extern-alien "try_acquire_gc_lock" (function int))) 1)
,@forms
(alien-funcall (extern-alien "release_gc_lock" (function void)))
t))
(defun sub-gc (gen)
(cond (*gc-inhibit*
@ -193,8 +199,6 @@ statistics are appended to it."
;; Let's make sure we're not interrupted and that none of
;; the deadline or deadlock detection stuff triggers.
(without-interrupts
(sb-thread::without-thread-waiting-for
(:already-without-interrupts t)
(let ((sb-impl::*deadline* nil)
(epoch *gc-epoch*))
(loop
@ -208,10 +212,9 @@ statistics are appended to it."
;; execute the remainder of the GC: stopping the
;; world with interrupts disabled is the mother of
;; all critical sections.
(cond ((sb-thread:with-mutex (*already-in-gc* :wait-p nil)
(cond ((try-acquire-gc-lock
(unsafe-clear-roots gen)
(gc-stop-the-world)
t)
(gc-stop-the-world))
;; Success! GC.
(perform-gc)
;; Return, but leave *gc-pending* as is: we
@ -238,7 +241,7 @@ statistics are appended to it."
;; runtime.
(when (and (eql gen 0)
(neq epoch *gc-pending*))
(return 0))))))))))
(return 0)))))))))
(defun post-gc ()
;; Outside the mutex, interrupts may be enabled: these may cause

View file

@ -76,7 +76,6 @@
;; SAVE-LISP-AND-DIE should have cleaned up, but there's a timing problem
;; with the finalizer thread, and I'm loathe to put in a SLEEP delay.
sb-thread::*joinable-threads*
sb-thread::*foreign-thread*
sb-thread::*all-threads*
sb-thread::*session*
sb-kernel::*gc-epoch*))

View file

@ -278,9 +278,6 @@ created and old ones may exit at any time."
(int-sap 0))
(sb-ext:define-load-time-global *initial-thread* nil)
;;; Always keep one preallocated foreign thread instance so that a non-lisp thread
;;; doesn't need to cons anything prior to release of the all_threads lock.
(sb-ext:define-load-time-global *foreign-thread* nil)
;;; *JOINABLE-THREADS* is a list of THREAD instances used only if #+pauseless-threadstart
;;; I had attempted to construct the list using the thread's memory to create cons
@ -327,7 +324,6 @@ created and old ones may exit at any time."
(init-thread-local-storage thread)
(setf *initial-thread* thread)
(setf *joinable-threads* nil)
#-sb-safepoint (setq *foreign-thread* (make-foreign-thread))
(setq *all-threads*
(avl-insert nil
(sb-thread::thread-primitive-thread sb-thread:*current-thread*)

View file

@ -78,9 +78,11 @@ pthread_key_t specials = 0;
#ifdef LISP_FEATURE_WIN32
CRITICAL_SECTION all_threads_lock;
static CRITICAL_SECTION recyclebin_lock;
static CRITICAL_SECTION in_gc_lock;
#else
pthread_mutex_t all_threads_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t recyclebin_lock = PTHREAD_MUTEX_INITIALIZER;
static pthread_mutex_t in_gc_lock = PTHREAD_MUTEX_INITIALIZER;
#endif
#endif
@ -253,6 +255,7 @@ void create_main_lisp_thread(lispobj function) {
#ifdef LISP_FEATURE_WIN32
InitializeCriticalSection(&all_threads_lock);
InitializeCriticalSection(&recyclebin_lock);
InitializeCriticalSection(&in_gc_lock);
#endif
struct thread *th = alloc_thread_struct(0, NO_TLS_VALUE_MARKER_WIDETAG);
if (!th || arch_os_thread_init(th)==0 || !init_shared_attr_object())
@ -316,8 +319,7 @@ void free_thread_struct(struct thread *th)
static void
init_new_thread(struct thread *th,
init_thread_data __attribute__((unused)) *scribble,
int guardp,
int retain_all_threads_lock)
int guardp)
{
int lock_ret;
@ -350,10 +352,7 @@ init_new_thread(struct thread *th,
lock_ret = thread_mutex_lock(&all_threads_lock);
gc_assert(lock_ret == 0);
link_thread(th);
if (!retain_all_threads_lock) {
lock_ret = thread_mutex_unlock(&all_threads_lock);
gc_assert(lock_ret == 0);
}
thread_mutex_unlock(&all_threads_lock);
/* Kludge: Changed the order of some steps between the safepoint/
* non-safepoint versions of this code. Can we unify this more?
@ -563,7 +562,7 @@ void* new_thread_trampoline(void* arg)
th->control_stack_end = (lispobj*)&arg + 1;
#endif
th->os_kernel_tid = get_nonzero_tid();
init_new_thread(th, SCRIBBLE, 0, 0);
init_new_thread(th, SCRIBBLE, 0);
// Passing the untagged pointer ensures 2 things:
// - that the pinning mechanism works as designed, and not just by accident.
// - that the initial stack does not contain a lisp pointer after it is not needed.
@ -586,8 +585,7 @@ void* new_thread_trampoline(void* arg)
lispobj function = th->no_tls_value_marker;
th->no_tls_value_marker = NO_TLS_VALUE_MARKER_WIDETAG;
init_new_thread(th, &scribble,
GUARD_CONTROL_STACK|GUARD_BINDING_STACK|GUARD_ALIEN_STACK,
0);
GUARD_CONTROL_STACK|GUARD_BINDING_STACK|GUARD_ALIEN_STACK);
funcall0(function);
unregister_thread(th, &scribble);
@ -709,16 +707,12 @@ attach_os_thread(init_thread_data *scribble)
th->control_stack_end = (void *) (((uintptr_t) stack_addr) + stack_size);
#endif
#ifdef LISP_FEATURE_SB_SAFEPOINT
const int retain_lock = 0;
#else
const int retain_lock = 1;
#endif
/* We don't protect the control stack when adopting a foreign thread
* because we wouldn't know where to put the guard */
init_new_thread(th, scribble,
/* recycled memory already had mprotect() done,
* so avoid 2 syscalls when possible */
recycled_memory ? 0 : GUARD_BINDING_STACK|GUARD_ALIEN_STACK,
retain_lock);
recycled_memory ? 0 : GUARD_BINDING_STACK|GUARD_ALIEN_STACK);
}
void
@ -817,13 +811,6 @@ callback_wrapper_trampoline(
}
}
// Balance out the mutex_lock in attach_os_thread()
void release_all_threads_lock()
{
if (thread_mutex_unlock(&all_threads_lock))
lose("ENTER-FOREIGN-CALLBACK bug");
}
#endif /* LISP_FEATURE_SB_THREAD */
/* this is called from any other thread to create the new one, and
@ -1137,6 +1124,14 @@ uword_t create_thread(struct thread_instance* instance, lispobj start_routine)
}
#endif
#ifdef LISP_FEATURE_WIN32
int try_acquire_gc_lock() { return TryEnterCriticalSection(&in_gc_lock); }
void release_gc_lock() { LeaveCriticalSection(&in_gc_lock); }
#else
int try_acquire_gc_lock() { return !pthread_mutex_trylock(&in_gc_lock); }
void release_gc_lock() { pthread_mutex_unlock(&in_gc_lock); }
#endif
/* stopping the world is a two-stage process. From this thread we signal
* all the others with SIG_STOP_FOR_GC. The handler for this signal does
* the usual pseudo-atomic checks (we don't want to stop a thread while