From: Nikodemus Siivola Date: Fri, 6 Apr 2007 09:58:08 +0000 (+0000) Subject: 1.0.4.30: make WITH-SPINLOCK-AND-WITHOUT-GCING inhibit interrupts as well X-Git-Url: http://repo.macrolet.net/gitweb/?a=commitdiff_plain;h=06c341ebab3da2e81effcaacc2235f2b1e8bfa82;p=sbcl.git 1.0.4.30: make WITH-SPINLOCK-AND-WITHOUT-GCING inhibit interrupts as well * Previously we could catch an interrupt while GC was inhibited, and hence any locks the interrupt handler was using could not know if they were being used with GC enabled or not => GC deadlocks. * Since this was detected by the runtime warning in WITH-INTERRUPTS about re-enabling interrupts while GC is inhibited keep it around in case we have more places this can happen. * Make MAYBE-HANDLE-PENDING-GC check for pending interrupts as well. * While we're at it, make WITH-SPINLOCK slightly safer: don't release locks we didn't obtain, and make us grab the lock inside the UWP. ...not 100% given in the presence of asynch unwinds, but better. * Also move *INTERRUPTS-ENABLED* and *INTERRUPT-PENDING* to SB-SYS. * Whitespace in WITHOUT-INTERRUPTS. --- diff --git a/NEWS b/NEWS index d67ccfe..bb3cb54 100644 --- a/NEWS +++ b/NEWS @@ -14,9 +14,9 @@ changes in sbcl-1.0.5 relative to sbcl-1.0.4: platforms. (thanks to James Anderson for the optimization hint) * bug fix: number of characters that can be written onto a single line in a file is unlimited. - * bug fix: GC deadlocks from asynchronous interrupts has been fixed - by disabling interrupts for the duration of any - SB-SYS:WITHOUT-GCING section. + * bug fix: some GC deadlocks caused by asynchronous interrupts have + been fixed by inhibiting interrupts for when GC is disbled. + * bug fix: GETHASH, PUTHASH, CLRHASH and REMHASH are now interrupt safe. changes in sbcl-1.0.4 relative to sbcl-1.0.3: * new platform: experimental support for x86-64/darwin (MacOS). diff --git a/package-data-list.lisp-expr b/package-data-list.lisp-expr index 2f05952..f4c0ad6 100644 --- a/package-data-list.lisp-expr +++ b/package-data-list.lisp-expr @@ -1928,6 +1928,7 @@ SB-KERNEL) have been undone, but probably more remain." "%PRIMITIVE" "%STANDARD-CHAR-P" "*FOREIGN-LOCK*" + "*INTERRUPTS-ENABLED*" "*INTERRUPT-PENDING*" "*LINKAGE-INFO*" "*LONG-SITE-NAME*" "*SHORT-SITE-NAME*" "*RUNTIME-DLHANDLE*" diff --git a/src/code/cold-init.lisp b/src/code/cold-init.lisp index 07af159..6a7123c 100644 --- a/src/code/cold-init.lisp +++ b/src/code/cold-init.lisp @@ -96,8 +96,8 @@ *gc-inhibit* t *gc-pending* nil #!+sb-thread *stop-for-gc-pending* #!+sb-thread nil - sb!unix::*interrupts-enabled* t - sb!unix::*interrupt-pending* nil + *interrupts-enabled* t + *interrupt-pending* nil *break-on-signals* nil *maximum-error-depth* 10 *current-error-depth* 0 diff --git a/src/code/early-impl.lisp b/src/code/early-impl.lisp index 3ab97bf..ea42696 100644 --- a/src/code/early-impl.lisp +++ b/src/code/early-impl.lisp @@ -33,8 +33,8 @@ ;; pseudo-atomicity too, but they handle it without ;; messing with special variables.) #!+(or x86 x86-64) *pseudo-atomic-bits* - sb!unix::*interrupts-enabled* - sb!unix::*interrupt-pending* + *interrupts-enabled* + *interrupt-pending* *free-interrupt-context-index* sb!vm::*allocation-pointer* sb!vm::*binding-stack-pointer* diff --git a/src/code/signal.lisp b/src/code/signal.lisp index 9d2290a..41be48f 100644 --- a/src/code/signal.lisp +++ b/src/code/signal.lisp @@ -49,23 +49,23 @@ userspace threads, in SBCL WITHOUT-INTERRUPTS does not inhibit scheduling of other threads." (let ((name (gensym "WITHOUT-INTERRUPTS-BODY-"))) `(flet ((,name () ,@body)) - (if *interrupts-enabled* - (unwind-protect - (let ((*interrupts-enabled* nil)) - (,name)) - ;; If we were interrupted in the protected section, then - ;; the interrupts are still blocked and it remains so - ;; until the pending interrupt is handled. - ;; - ;; If we were not interrupted in the protected section, - ;; but here, then even if the interrupt handler enters - ;; another WITHOUT-INTERRUPTS, the pending interrupt will - ;; be handled immediately upon exit from said - ;; WITHOUT-INTERRUPTS, so it is as if nothing has - ;; happened. - (when *interrupt-pending* - (receive-pending-interrupt))) - (,name))))) + (if *interrupts-enabled* + (unwind-protect + (let ((*interrupts-enabled* nil)) + (,name)) + ;; If we were interrupted in the protected section, then + ;; the interrupts are still blocked and it remains so + ;; until the pending interrupt is handled. + ;; + ;; If we were not interrupted in the protected section, + ;; but here, then even if the interrupt handler enters + ;; another WITHOUT-INTERRUPTS, the pending interrupt will + ;; be handled immediately upon exit from said + ;; WITHOUT-INTERRUPTS, so it is as if nothing has + ;; happened. + (when *interrupt-pending* + (receive-pending-interrupt))) + (,name))))) (sb!xc:defmacro with-interrupts (&body body) #!+sb-doc diff --git a/src/code/target-hash-table.lisp b/src/code/target-hash-table.lisp index 8865c5a..18a50c9 100644 --- a/src/code/target-hash-table.lisp +++ b/src/code/target-hash-table.lisp @@ -21,8 +21,10 @@ (defmacro with-spinlock-and-without-gcing ((spinlock) &body body) #!-sb-thread (declare (ignore spinlock)) - (with-unique-names (old-gc-inhibit) + (with-unique-names (old-gc-inhibit old-interrupts-enabled) `(let ((,old-gc-inhibit *gc-inhibit*) + (,old-interrupts-enabled *interrupts-enabled*) + (*interrupts-enabled* nil) (*gc-inhibit* t)) (unwind-protect (progn @@ -31,8 +33,10 @@ ,@body) #!+sb-thread (sb!thread::release-spinlock ,spinlock) - (let ((*gc-inhibit* ,old-gc-inhibit)) - ;; the test is racy, but it can err only on the overeager side + (let ((*interrupts-enabled* ,old-interrupts-enabled) + (*gc-inhibit* ,old-gc-inhibit)) + ;; the test is racy, but it can err only on the overeager + ;; side (sb!kernel::maybe-handle-pending-gc)))))) (eval-when (:compile-toplevel :load-toplevel :execute) diff --git a/src/code/target-thread.lisp b/src/code/target-thread.lisp index 57747fc..42ad3dc 100644 --- a/src/code/target-thread.lisp +++ b/src/code/target-thread.lisp @@ -208,12 +208,15 @@ in future versions." (sb!vm::%instance-set spinlock 2 0)) (defmacro with-spinlock ((spinlock) &body body) - (sb!int:with-unique-names (lock) - `(let ((,lock ,spinlock)) - (get-spinlock ,lock) + (sb!int:with-unique-names (lock got-it) + `(let ((,lock ,spinlock) + (,got-it nil)) (unwind-protect - (progn ,@body) - (release-spinlock ,lock))))) + (progn + (setf ,got-it (get-spinlock ,lock)) + (locally ,@body)) + (when ,got-it + (release-spinlock ,lock)))))) ;;;; mutexes diff --git a/src/code/toplevel.lisp b/src/code/toplevel.lisp index ef3f173..416949b 100644 --- a/src/code/toplevel.lisp +++ b/src/code/toplevel.lisp @@ -26,8 +26,8 @@ ;;; FIXME: These could be converted to DEFVARs. (declaim (special #!+(or x86 x86-64) *pseudo-atomic-bits* - sb!unix::*interrupts-enabled* - sb!unix::*interrupt-pending* + *interrupts-enabled* + *interrupt-pending* *type-system-initialized*)) (defvar *cold-init-complete-p*) diff --git a/version.lisp-expr b/version.lisp-expr index ecef46b..562aa24 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".) -"1.0.4.29" +"1.0.4.30"