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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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"