X-Git-Url: http://repo.macrolet.net/gitweb/?a=blobdiff_plain;f=src%2Fcode%2Ftarget-package.lisp;h=ccaa5bbe8c0b2da277ae83b291544f08f6ae93cf;hb=9152e162ab62d7027a4403fb9b461c31847649a2;hp=a8598ad85c7b6449c7db35ba3c24672aa8c856f0;hpb=8cd0537738745bc866b69c4132cac6c881d67405;p=sbcl.git diff --git a/src/code/target-package.lisp b/src/code/target-package.lisp index a8598ad..ccaa5bb 100644 --- a/src/code/target-package.lisp +++ b/src/code/target-package.lisp @@ -28,6 +28,56 @@ (!cold-init-forms (/show0 "entering !PACKAGE-COLD-INIT")) +;;;; Thread safety +;;;; +;;;; ...this could still use work, but the basic idea is: +;;;; +;;;; *PACKAGE-GRAPH-LOCK* is held via WITH-PACKAGE-GRAPH while working on +;;;; package graph, including package -> package links, and interning and +;;;; uninterning symbols. +;;;; +;;;; Hash-table lock on *PACKAGE-NAMES* is held via WITH-PACKAGE-NAMES while +;;;; frobbing name -> package associations. +;;;; +;;;; There should be no deadlocks due to ordering issues between these two, as +;;;; the latter is only held over operations guaranteed to terminate in finite +;;;; time. +;;;; +;;;; Errors may be signalled while holding on to the *PACKAGE-GRAPH-LOCK*, +;;;; which can still lead to pretty damned inconvenient situations -- but +;;;; since FIND-PACKAGE, FIND-SYMBOL from other threads isn't blocked by this, +;;;; the situation isn't *quite* hopeless. +;;;; +;;;; A better long-term solution seems to be in splitting the granularity of +;;;; the *PACKAGE-GRAPH-LOCK* down: for interning a per-package lock should be +;;;; sufficient, though interaction between parallel intern and use-package +;;;; needs to be considered with some care. + +(defvar *package-graph-lock*) +(!cold-init-forms + (setf *package-graph-lock* (sb!thread:make-mutex :name "Package Graph Lock"))) + +(defun call-with-package-graph (function) + (declare (function function)) + ;; 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-graph-lock*) + (funcall function))) + +;;; a map from package names to packages +(defvar *package-names*) +(declaim (type hash-table *package-names*)) +(!cold-init-forms + (setf *package-names* (make-hash-table :test 'equal :synchronized t))) + +(defmacro with-package-names ((names &key) &body body) + `(let ((,names *package-names*)) + (with-locked-hash-table (,names) + ,@body))) + ;;;; PACKAGE-HASHTABLE stuff (def!method print-object ((table package-hashtable) stream) @@ -312,12 +362,6 @@ error if any of PACKAGES is not a valid package designator." ;;; FIXME: should be declared of type PACKAGE, with no NIL init form, ;;; after I get around to cleaning up DOCUMENTATION -;;; a map from package names to packages -(defvar *package-names*) -(declaim (type hash-table *package-names*)) -(!cold-init-forms - (setf *package-names* (make-hash-table :test 'equal))) - ;;; This magical variable is T during initialization so that ;;; USE-PACKAGE's of packages that don't yet exist quietly win. Such ;;; packages are thrown onto the list *DEFERRED-USE-PACKAGES* so that @@ -375,7 +419,7 @@ error if any of PACKAGES is not a valid package designator." ;;; Make a package name into a simple-string. (defun package-namify (n) - (stringify-name n "package")) + (stringify-package-designator n)) ;;; ANSI specifies (in the definition of DELETE-PACKAGE) that PACKAGE-NAME ;;; returns NIL (not an error) for a deleted package, so this is a special @@ -458,10 +502,10 @@ error if any of PACKAGES is not a valid package designator." `(let* ((,vec (package-hashtable-table ,table)) (,hash (package-hashtable-hash ,table)) (,len (length ,vec)) - (,h2 (1+ (the index (rem (the index ,sxhash) + (,h2 (1+ (the index (rem (the hash ,sxhash) (the index (- ,len 2))))))) (declare (type index ,len ,h2)) - (prog ((,index-var (rem (the index ,sxhash) ,len)) + (prog ((,index-var (rem (the hash ,sxhash) ,len)) ,symbol-var ,ehash) (declare (type (or index null) ,index-var)) LOOP @@ -492,7 +536,8 @@ error if any of PACKAGES is not a valid package designator." (let* ((length (length string)) (hash (%sxhash-simple-string string)) (ehash (entry-hash length hash))) - (declare (type index length hash)) + (declare (type index length) + (type hash hash)) (with-symbol (index symbol table string length hash ehash) (setf (aref (package-hashtable-hash table) index) 1) (setf (aref (package-hashtable-table table) index) nil) @@ -508,18 +553,20 @@ error if any of PACKAGES is not a valid package designator." (when (< used (truncate size 4)) (resize-package-hashtable table (* used 2))))) -;;; Enter any new NICKNAMES for PACKAGE into *PACKAGE-NAMES*. -;;; If there is a conflict then give the user a chance to do -;;; something about it. -(defun enter-new-nicknames (package nicknames) +;;; Enter any new NICKNAMES for PACKAGE into *PACKAGE-NAMES*. If there is a +;;; conflict then give the user a chance to do something about it. Caller is +;;; responsible for having acquired the mutex via WITH-PACKAGES. +(defun %enter-new-nicknames (package nicknames) (declare (type list nicknames)) (dolist (n nicknames) (let* ((n (package-namify n)) - (found (gethash n *package-names*))) - (cond ((not found) - (setf (gethash n *package-names*) package) - (push n (package-%nicknames package))) - ((eq found package)) + (found (with-package-names (names) + (or (gethash n names) + (progn + (setf (gethash n names) package) + (push n (package-%nicknames package)) + package))))) + (cond ((eq found package)) ((string= (the string (package-%name found)) n) (cerror "Ignore this nickname." 'simple-package-error @@ -540,43 +587,46 @@ error if any of PACKAGES is not a valid package designator." (external-symbols 10)) #!+sb-doc #.(format nil - "Make a new package having the specified NAME, NICKNAMES, and - USE list. :INTERNAL-SYMBOLS and :EXTERNAL-SYMBOLS are - estimates for the number of internal and external symbols which - will ultimately be present in the package. The default value of - USE is implementation-dependent, and in this implementation - it is ~S." - *default-package-use-list*) - - ;; Check for package name conflicts in name and nicknames, then - ;; make the package. - (when (find-package name) - ;; ANSI specifies that this error is correctable. - (cerror "Leave existing package alone." - "A package named ~S already exists" name)) - (let* ((name (package-namify name)) - (package (internal-make-package - :%name name - :internal-symbols (make-or-remake-package-hashtable - internal-symbols) - :external-symbols (make-or-remake-package-hashtable - external-symbols)))) - - ;; Do a USE-PACKAGE for each thing in the USE list so that checking for - ;; conflicting exports among used packages is done. - (if *in-package-init* - (push (list use package) *!deferred-use-packages*) - (use-package use package)) - - ;; FIXME: ENTER-NEW-NICKNAMES can fail (ERROR) if nicknames are illegal, - ;; which would leave us with possibly-bad side effects from the earlier - ;; USE-PACKAGE (e.g. this package on the used-by lists of other packages, - ;; but not in *PACKAGE-NAMES*, and possibly import side effects too?). - ;; Perhaps this can be solved by just moving ENTER-NEW-NICKNAMES before - ;; USE-PACKAGE, but I need to check what kinds of errors can be caused by - ;; USE-PACKAGE, too. - (enter-new-nicknames package nicknames) - (setf (gethash name *package-names*) package))) + "Make a new package having the specified NAME, NICKNAMES, and USE +list. :INTERNAL-SYMBOLS and :EXTERNAL-SYMBOLS are estimates for the number of +internal and external symbols which will ultimately be present in the package. +The default value of USE is implementation-dependent, and in this +implementation it is ~S." *default-package-use-list*) + (prog (clobber) + :restart + (when (find-package name) + ;; ANSI specifies that this error is correctable. + (cerror "Clobber existing package." + "A package named ~S already exists" name) + (setf clobber t)) + (with-packages () + ;; Check for race, signal the error outside the lock. + (when (and (not clobber) (find-package name)) + (go :restart)) + (let* ((name (package-namify name)) + (package (internal-make-package + :%name name + :internal-symbols (make-or-remake-package-hashtable + internal-symbols) + :external-symbols (make-or-remake-package-hashtable + external-symbols)))) + + ;; Do a USE-PACKAGE for each thing in the USE list so that checking for + ;; conflicting exports among used packages is done. + (if *in-package-init* + (push (list use package) *!deferred-use-packages*) + (use-package use package)) + + ;; FIXME: ENTER-NEW-NICKNAMES can fail (ERROR) if nicknames are illegal, + ;; which would leave us with possibly-bad side effects from the earlier + ;; USE-PACKAGE (e.g. this package on the used-by lists of other packages, + ;; but not in *PACKAGE-NAMES*, and possibly import side effects too?). + ;; Perhaps this can be solved by just moving ENTER-NEW-NICKNAMES before + ;; USE-PACKAGE, but I need to check what kinds of errors can be caused by + ;; USE-PACKAGE, too. + (%enter-new-nicknames package nicknames) + (return (setf (gethash name *package-names*) package)))) + (bug "never"))) ;;; Change the name if we can, blast any old nicknames and then ;;; add in any new ones. @@ -587,103 +637,115 @@ error if any of PACKAGES is not a valid package designator." ;;; the package name if NAME is the same package that's referred to by PACKAGE. ;;; If it's a *different* package, we should probably signal an error. ;;; (perhaps (ERROR 'ANSI-WEIRDNESS ..):-) -(defun rename-package (package name &optional (nicknames ())) +(defun rename-package (package-designator name &optional (nicknames ())) #!+sb-doc "Changes the name and nicknames for a package." - (let* ((package (find-undeleted-package-or-lose package)) - (name (package-namify name)) - (found (find-package name)) - (nicks (mapcar #'string nicknames))) - (unless (or (not found) (eq found package)) - (error 'simple-package-error - :package name - :format-control "A package named ~S already exists." - :format-arguments (list name))) - (with-single-package-locked-error () - (unless (and (string= name (package-name package)) - (null (set-difference nicks (package-nicknames package) - :test #'string=))) - (assert-package-unlocked package "rename as ~A~@[ with nickname~P ~ + (let ((package nil)) + (tagbody :restart + (setq package (find-undeleted-package-or-lose package-designator)) + (let* ((name (package-namify name)) + (found (find-package name)) + (nicks (mapcar #'string nicknames))) + (unless (or (not found) (eq found package)) + (error 'simple-package-error + :package name + :format-control "A package named ~S already exists." + :format-arguments (list name))) + (with-single-package-locked-error () + (unless (and (string= name (package-name package)) + (null (set-difference nicks (package-nicknames package) + :test #'string=))) + (assert-package-unlocked package "rename as ~A~@[ with nickname~P ~ ~{~A~^, ~}~]" - name (length nicks) nicks)) - ;; do the renaming - (remhash (package-%name package) *package-names*) - (dolist (n (package-%nicknames package)) - (remhash n *package-names*)) - (setf (package-%name package) name - (gethash name *package-names*) package - (package-%nicknames package) ()) - (enter-new-nicknames package nicknames)) + name (length nicks) nicks)) + (with-package-names (names) + ;; Check for race conditions now that we have the lock. + (unless (eq package (find-package package-designator)) + (go :restart)) + ;; Do the renaming. + (remhash (package-%name package) names) + (dolist (n (package-%nicknames package)) + (remhash n names)) + (setf (package-%name package) name + (gethash name names) package + (package-%nicknames package) ())) + (%enter-new-nicknames package nicknames)))) package)) (defun delete-package (package-designator) #!+sb-doc "Delete the package designated by PACKAGE-DESIGNATOR from the package system data structures." - (let ((package (if (packagep package-designator) - package-designator - (find-package package-designator)))) - (cond ((not package) - ;; This continuable error is required by ANSI. - (cerror - "Return ~S." - (make-condition - 'simple-package-error - :package package-designator - :format-control "There is no package named ~S." - :format-arguments (list package-designator)) - nil)) - ((not (package-name package)) ; already deleted - nil) - (t - (with-single-package-locked-error - (:package package "deleting package ~A" package) - (let ((use-list (package-used-by-list package))) - (when use-list - ;; This continuable error is specified by ANSI. - (cerror - "Remove dependency in other packages." - (make-condition - 'simple-package-error - :package package - :format-control - "~@" - :format-arguments (list (package-name package) - (length use-list) - (mapcar #'package-name use-list)))) - (dolist (p use-list) - (unuse-package package p)))) - (dolist (used (package-use-list package)) - (unuse-package used package)) - (do-symbols (sym package) - (unintern sym package)) - (remhash (package-name package) *package-names*) - (dolist (nick (package-nicknames package)) - (remhash nick *package-names*)) - (setf (package-%name package) nil - ;; Setting PACKAGE-%NAME to NIL is required in order to - ;; make PACKAGE-NAME return NIL for a deleted package as - ;; ANSI requires. Setting the other slots to NIL - ;; and blowing away the PACKAGE-HASHTABLES is just done - ;; for tidiness and to help the GC. - (package-%nicknames package) nil - (package-%use-list package) nil - (package-tables package) nil - (package-%shadowing-symbols package) nil - (package-internal-symbols package) - (make-or-remake-package-hashtable 0) - (package-external-symbols package) - (make-or-remake-package-hashtable 0)) - t))))) + (tagbody :restart + (let ((package (find-package package-designator))) + (cond ((not package) + ;; This continuable error is required by ANSI. + (cerror + "Return ~S." + (make-condition + 'simple-package-error + :package package-designator + :format-control "There is no package named ~S." + :format-arguments (list package-designator)) + (return-from delete-package nil))) + ((not (package-name package)) ; already deleted + (return-from delete-package nil)) + (t + (with-single-package-locked-error + (:package package "deleting package ~A" package) + (let ((use-list (package-used-by-list package))) + (when use-list + ;; This continuable error is specified by ANSI. + (cerror + "Remove dependency in other packages." + (make-condition + 'simple-package-error + :package package + :format-control + "~@" + :format-arguments (list (package-name package) + (length use-list) + (mapcar #'package-name use-list)))) + (dolist (p use-list) + (unuse-package package p)))) + (with-package-graph () + ;; Check for races, restart if necessary. + (let ((package2 (find-package package-designator))) + (when (or (neq package package2) (package-used-by-list package2)) + (go :restart))) + (dolist (used (package-use-list package)) + (unuse-package used package)) + (do-symbols (sym package) + (unintern sym package)) + (with-package-names (names) + (remhash (package-name package) names) + (dolist (nick (package-nicknames package)) + (remhash nick names)) + (setf (package-%name package) nil + ;; Setting PACKAGE-%NAME to NIL is required in order to + ;; make PACKAGE-NAME return NIL for a deleted package as + ;; ANSI requires. Setting the other slots to NIL + ;; and blowing away the PACKAGE-HASHTABLES is just done + ;; for tidiness and to help the GC. + (package-%nicknames package) nil)) + (setf (package-%use-list package) nil + (package-tables package) nil + (package-%shadowing-symbols package) nil + (package-internal-symbols package) + (make-or-remake-package-hashtable 0) + (package-external-symbols package) + (make-or-remake-package-hashtable 0))) + (return-from delete-package t))))))) (defun list-all-packages () #!+sb-doc "Return a list of all existing packages." (let ((res ())) - (maphash (lambda (k v) - (declare (ignore k)) - (pushnew v res)) - *package-names*) + (with-package-names (names) + (maphash (lambda (k v) + (declare (ignore k)) + (pushnew v res)) + names)) res)) (defun intern (name &optional (package (sane-package))) @@ -723,17 +785,26 @@ error if any of PACKAGES is not a valid package designator." (cond (where (values symbol where)) (t - (let ((symbol-name (subseq name 0 length))) - (with-single-package-locked-error - (:package package "interning ~A" symbol-name) - (let ((symbol (make-symbol symbol-name))) - (%set-symbol-package symbol package) - (cond ((eq package *keyword-package*) - (add-symbol (package-external-symbols package) symbol) - (%set-symbol-value symbol symbol)) - (t - (add-symbol (package-internal-symbols package) symbol))) - (values symbol nil)))))))) + ;; Let's try again with a lock: the common case has the + ;; symbol already interned, handled by the first leg of the + ;; COND, but in case another thread is interning in + ;; parallel we need to check after grabbing the lock. + (with-package-graph () + (setf (values symbol where) (find-symbol* name length package)) + (if where + (values symbol where) + (let ((symbol-name (subseq name 0 length))) + (with-single-package-locked-error + (:package package "interning ~A" symbol-name) + (let ((symbol (make-symbol symbol-name))) + (%set-symbol-package symbol package) + (cond + ((eq package *keyword-package*) + (%set-symbol-value symbol symbol) + (add-symbol (package-external-symbols package) symbol)) + (t + (add-symbol (package-internal-symbols package) symbol))) + (values symbol nil)))))))))) ;;; Check internal and external symbols, then scan down the list ;;; of hashtables for inherited symbols. @@ -742,7 +813,7 @@ error if any of PACKAGES is not a valid package designator." (type index length)) (let* ((hash (%sxhash-simple-substring string length)) (ehash (entry-hash length hash))) - (declare (type index hash ehash)) + (declare (type hash hash ehash)) (with-symbol (found symbol (package-internal-symbols package) string length hash ehash) (when found @@ -781,11 +852,23 @@ error if any of PACKAGES is not a valid package designator." (let* ((length (length string)) (hash (%sxhash-simple-string string)) (ehash (entry-hash length hash))) - (declare (type index length hash)) + (declare (type index length) + (type hash hash)) (with-symbol (found symbol (package-external-symbols package) string length hash ehash) (values symbol found)))) +(defun print-symbol-with-prefix (stream symbol colon at) + #!+sb-doc + "For use with ~/: Write SYMBOL to STREAM as if it is not accessible from + the current package." + (declare (ignore colon at)) + ;; Only keywords should be accessible from the keyword package, and + ;; keywords are always printed with colons, so this guarantees that the + ;; symbol will not be printed without a prefix. + (let ((*package* *keyword-package*)) + (write symbol :stream stream :escape t))) + (define-condition name-conflict (reference-condition package-error) ((function :initarg :function :reader name-conflict-function) (datum :initarg :datum :reader name-conflict-datum) @@ -794,7 +877,8 @@ error if any of PACKAGES is not a valid package designator." (:report (lambda (c s) (format s "~@<~S ~S causes name-conflicts in ~S between the ~ - following symbols:~2I~@:_~{~S~^, ~}~:@>" + following symbols:~2I~@:_~ + ~{~/sb-impl::print-symbol-with-prefix/~^, ~}~:@>" (name-conflict-function c) (name-conflict-datum c) (package-error-package c) @@ -804,7 +888,7 @@ error if any of PACKAGES is not a valid package designator." (restart-case (error 'name-conflict :package package :symbols symbols :function function :datum datum) - (resolve-conflict (s) + (resolve-conflict (chosen-symbol) :report "Resolve conflict." :interactive (lambda () @@ -812,7 +896,9 @@ error if any of PACKAGES is not a valid package designator." (nlen (length (write-to-string len :base 10))) (*print-pretty* t)) (format *query-io* "~&~@