From: Gabor Melis <mega@hotpop.com>
Date: Mon, 16 Feb 2009 22:26:25 +0000 (+0000)
Subject: 1.0.25.51: use WITH-RECURSIVE-SYSTEM-SPINLOCK
X-Git-Url: http://repo.macrolet.net/gitweb/?a=commitdiff_plain;h=78c7dbe937e6783e8da6f7a39c3eba87294d197c;p=sbcl.git

1.0.25.51: use WITH-RECURSIVE-SYSTEM-SPINLOCK

... instead of WITH-RECURSIVE-SPINLOCK because it's possible to
deadlock due to lock ordering with sufficiently unlucky interrupts as
demonstrated by test (:timer :parallel-unschedule) with low
probability.

This affects hash tables and some pcl locks.

Also, use WITH-RECURSIVE-MUTEX for packages.

Not a spinlock becuase it can be held for a long time and not a system
lock (i.e. with WITHOUT-INTERRUPTS) because conflicts are signalled
while holding the lock which I think this warrants a FIXME.
---

diff --git a/NEWS b/NEWS
index 3d91a48..7ce43b4 100644
--- a/NEWS
+++ b/NEWS
@@ -16,6 +16,8 @@ changes in sbcl-1.0.26 relative to 1.0.25:
     memory, stack, alien stack, binding stack, encountering a memory
     fault, etc. In the absence of --lose-on-corruption a warning is
     printed to stderr.
+  * enhancement: detect binding stack exhaustion
+  * enhancement: detect alien stack exhaustion on x86/x86-64
   * improvement: generally more stable and reliable interrupt handling
   * improvement: there is a per thread interruption queue,
     interruptions are executed in order of arrival
@@ -37,6 +39,8 @@ changes in sbcl-1.0.26 relative to 1.0.25:
   * bug fix: fix deadlocks related to starting threads
   * bug fix: fix deadlines on locks on futex platforms
   * bug fix: restore errno in signal handlers
+  * bug fix: fix deadlocks related to hash tables
+  * bug fix: fix deadlocks in pcl
 
 changes in sbcl-1.0.25 relative to 1.0.24:
   * incompatible change: SB-INTROSPECT:FUNCTION-ARGLIST is deprecated, to be
diff --git a/src/code/hash-table.lisp b/src/code/hash-table.lisp
index ca66744..784bc11 100644
--- a/src/code/hash-table.lisp
+++ b/src/code/hash-table.lisp
@@ -147,5 +147,6 @@ unspecified."
   ;; Needless to say, this also excludes some internal bits, but
   ;; getting there is too much detail when "unspecified" says what
   ;; is important -- unpredictable, but harmless.
-  `(sb!thread::with-recursive-spinlock ((hash-table-spinlock ,hash-table))
+  `(sb!thread::with-recursive-system-spinlock
+       ((hash-table-spinlock ,hash-table))
      ,@body))
diff --git a/src/code/target-package.lisp b/src/code/target-package.lisp
index 4fa7582..01ed836 100644
--- a/src/code/target-package.lisp
+++ b/src/code/target-package.lisp
@@ -51,10 +51,15 @@
 (defvar *package-lock*)
 
 (!cold-init-forms
- (setf *package-lock* (sb!thread::make-spinlock :name "Package Lock")))
+ (setf *package-lock* (sb!thread:make-mutex :name "Package Lock")))
 
 (defmacro with-packages ((&key) &body forms)
-  `(sb!thread::with-recursive-spinlock (*package-lock*)
+  ;; FIXME: Since name conflicts can be signalled while holding the
+  ;; mutex, user code can be run leading to lock ordering problems.
+  ;;
+  ;; This used to be a spinlock, but there it can be held for a long
+  ;; time while the debugger waits for user input.
+  `(sb!thread:with-recursive-lock (*package-lock*)
      ,@forms))
 
 ;;; Make a package hashtable having a prime number of entries at least
diff --git a/src/pcl/std-class.lisp b/src/pcl/std-class.lisp
index bf80f1b..b077674 100644
--- a/src/pcl/std-class.lisp
+++ b/src/pcl/std-class.lisp
@@ -181,13 +181,13 @@
 (defmethod add-direct-method :around ((specializer specializer) method)
   ;; All the actions done under this lock are done in an order
   ;; that is safe to unwind at any point.
-  (sb-thread::with-recursive-spinlock (*specializer-lock*)
+  (sb-thread::with-recursive-system-spinlock (*specializer-lock*)
     (call-next-method)))
 
 (defmethod remove-direct-method :around ((specializer specializer) method)
   ;; All the actions done under this lock are done in an order
   ;; that is safe to unwind at any point.
-  (sb-thread::with-recursive-spinlock (*specializer-lock*)
+  (sb-thread::with-recursive-system-spinlock (*specializer-lock*)
     (call-next-method)))
 
 (defmethod add-direct-method ((specializer class) (method method))
diff --git a/version.lisp-expr b/version.lisp-expr
index 1bfc001..7382e83 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.25.50"
+"1.0.25.51"