1.0.41.43: fix for RENAME-PACKAGE's return value
[sbcl.git] / src / code / target-package.lisp
index 890bd11..ccaa5bb 100644 (file)
 (!cold-init-forms
   (/show0 "entering !PACKAGE-COLD-INIT"))
 \f
+;;;; 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)))
+\f
 ;;;; PACKAGE-HASHTABLE stuff
 
 (def!method print-object ((table package-hashtable) stream)
 ;;; core image
 (defconstant +package-hashtable-image-load-factor+ 0.5)
 
-;;; All destructive package modifications are serialized on this lock,
-;;; plus iterations on *PACKAGE-NAMES*.
-(defvar *package-lock*)
-
-(!cold-init-forms
- (setf *package-lock* (sb!thread::make-spinlock :name "Package Lock")))
-
-(defmacro with-packages ((&key) &body forms)
-  `(sb!thread::with-recursive-spinlock (*package-lock*)
-     ,@forms))
-
 ;;; Make a package hashtable having a prime number of entries at least
 ;;; as great as (/ SIZE +PACKAGE-REHASH-THRESHOLD+). If RES is supplied,
 ;;; then it is destructively modified to produce the result. This is
@@ -323,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
@@ -469,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
@@ -503,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)
@@ -519,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)))))
 \f
-;;; 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
@@ -556,36 +592,41 @@ 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*)
-  (with-packages ()
-    ;; 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))))
+  (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.
@@ -596,106 +637,115 @@ implementation it is ~S." *default-package-use-list*)
 ;;; 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."
-  (with-packages ()
-    (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))
-      package)))
+                                    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."
-  (with-packages ()
-    (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
-                     "~@<Package ~S is used by package~P:~2I~_~S~@:>"
-                     :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
+                      "~@<Package ~S is used by package~P:~2I~_~S~@:>"
+                      :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 ()))
-    (with-packages ()
+    (with-package-names (names)
       (maphash (lambda (k v)
                  (declare (ignore k))
                  (pushnew v res))
-               *package-names*))
+               names))
     res))
 \f
 (defun intern (name &optional (package (sane-package)))
@@ -739,7 +789,7 @@ implementation it is ~S." *default-package-use-list*)
            ;; 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-packages ()
+           (with-package-graph ()
              (setf (values symbol where) (find-symbol* name length package))
              (if where
                  (values symbol where)
@@ -750,8 +800,8 @@ implementation it is ~S." *default-package-use-list*)
                        (%set-symbol-package symbol package)
                        (cond
                          ((eq package *keyword-package*)
-                          (add-symbol (package-external-symbols package) symbol)
-                          (%set-symbol-value symbol symbol))
+                          (%set-symbol-value symbol symbol)
+                          (add-symbol (package-external-symbols package) symbol))
                          (t
                           (add-symbol (package-internal-symbols package) symbol)))
                        (values symbol nil))))))))))
@@ -763,7 +813,7 @@ implementation it is ~S." *default-package-use-list*)
            (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
@@ -802,7 +852,8 @@ implementation it is ~S." *default-package-use-list*)
   (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))))
@@ -837,7 +888,7 @@ implementation it is ~S." *default-package-use-list*)
   (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 ()
@@ -858,102 +909,32 @@ implementation it is ~S." *default-package-use-list*)
            (let ((i (parse-integer (read-line *query-io*) :junk-allowed t)))
              (when (and i (<= 1 i len))
                (return (list (nth (1- i) symbols))))))))
-      (multiple-value-bind (symbol status)
-          (find-symbol (symbol-name s) package)
-        (declare (ignore status)) ; FIXME: is that true?
-        (case function
-          ((export)
-           (if (eq symbol s)
-               (shadow symbol package)
-               (unintern symbol package)))
-          ((unintern)
-           (shadowing-import s package))
-          ((import)
-           (if (eq symbol s)
-               nil ; do nothing
-               (shadowing-import s package)))
-          ((use-package)
-           (if (eq symbol s)
-               (shadow s package)
-               (shadowing-import s package))))))))
-
-#+nil ; this solution gives a variable number of restarts instead, but
-      ; no good way of programmatically choosing between them.
-(defun name-conflict (package function datum &rest symbols)
-  (let ((condition (make-condition 'name-conflict
-                                   :package package :symbols symbols
-                                   :function function :datum datum)))
-    ;; this is a gross violation of modularity, but I can't see any
-    ;; other way to have a variable number of restarts.
-    (let ((*restart-clusters*
-           (cons
-            (mapcan
-             (lambda (s)
-               (multiple-value-bind (accessible-symbol status)
-                   (find-symbol (symbol-name s) package)
-                 (cond
-                   ;; difficult case
-                   ((eq s accessible-symbol)
-                    (ecase status
-                      ((:inherited)
-                       (list (make-restart
-                              :name (make-symbol "SHADOWING-IMPORT")
-                              :function (lambda ()
-                                          (shadowing-import s package)
-                                          (return-from name-conflict))
-                              :report-function
-                              (lambda (stream)
-                                (format stream "Shadowing-import ~S into ~A."
-                                        s (package-%name package))))))
-                      ((:internal :external)
-                       (aver (= (length symbols) 2))
-                       ;; ARGH! FIXME: this unintern restart can
-                       ;; _still_ leave the system in an
-                       ;; unsatisfactory state: if the symbol is a
-                       ;; external symbol of a package which is
-                       ;; already used by this package, and has also
-                       ;; been imported, then uninterning it from this
-                       ;; package will still leave it visible!
-                       ;;
-                       ;; (DEFPACKAGE "FOO" (:EXPORT "SYM"))
-                       ;; (DEFPACKAGE "BAR" (:EXPORT "SYM"))
-                       ;; (DEFPACKAGE "BAZ" (:USE "FOO"))
-                       ;; (IMPORT 'FOO:SYM "BAZ")
-                       ;; (USE-PACKAGE "BAR" "BAZ")
-                       ;;
-                       ;; Now (UNINTERN 'FOO:SYM "BAZ") doesn't
-                       ;; resolve the conflict. :-(
-                       ;;
-                       ;; -- CSR, 2004-10-20
-                       (list (make-restart
-                              :name (make-symbol "UNINTERN")
-                              :function (lambda ()
-                                          (unintern s package)
-                                          (import
-                                           (find s symbols :test-not #'eq)
-                                           package)
-                                          (return-from name-conflict))
-                              :report-function
-                              (lambda (stream)
-                                (format stream
-                                        "Unintern ~S from ~A and import ~S."
-                                        s
-                                        (package-%name package)
-                                        (find s symbols :test-not #'eq))))))))
-                   (t (list (make-restart
-                             :name (make-symbol "SHADOWING-IMPORT")
-                             :function (lambda ()
-                                         (shadowing-import s package)
-                                         (return-from name-conflict))
-                             :report-function
-                             (lambda (stream)
-                               (format stream "Shadowing-import ~S into ~A."
-                                       s (package-%name package)))))))))
-             symbols)
-            *restart-clusters*)))
-      (with-condition-restarts condition (car *restart-clusters*)
-        (with-simple-restart (abort "Leave action undone.")
-          (error condition))))))
+      (multiple-value-bind (package-symbol status)
+          (find-symbol (symbol-name chosen-symbol) package)
+        (let* ((accessiblep status)     ; never NIL here
+               (presentp (and accessiblep
+                              (not (eq :inherited status)))))
+          (ecase function
+            ((unintern)
+             (if presentp
+                 (if (eq package-symbol chosen-symbol)
+                     (shadow (list package-symbol) package)
+                     (shadowing-import (list chosen-symbol) package))
+                 (shadowing-import (list chosen-symbol) package)))
+            ((use-package export)
+             (if presentp
+                 (if (eq package-symbol chosen-symbol)
+                     (shadow (list package-symbol) package) ; CLHS 11.1.1.2.5
+                     (if (eq (symbol-package package-symbol) package)
+                         (unintern package-symbol package) ; CLHS 11.1.1.2.5
+                         (shadowing-import (list chosen-symbol) package)))
+                 (shadowing-import (list chosen-symbol) package)))
+            ((import)
+             (if presentp
+                 (if (eq package-symbol chosen-symbol)
+                     nil                ; re-importing the same symbol
+                     (shadowing-import (list chosen-symbol) package))
+                 (shadowing-import (list chosen-symbol) package)))))))))
 
 ;;; If we are uninterning a shadowing symbol, then a name conflict can
 ;;; result, otherwise just nuke the symbol.
@@ -962,7 +943,7 @@ implementation it is ~S." *default-package-use-list*)
   "Makes SYMBOL no longer present in PACKAGE. If SYMBOL was present then T is
 returned, otherwise NIL. If PACKAGE is SYMBOL's home package, then it is made
 uninterned."
-  (with-packages ()
+  (with-package-graph ()
     (let* ((package (find-undeleted-package-or-lose package))
            (name (symbol-name symbol))
            (shadowing-symbols (package-%shadowing-symbols package)))
@@ -1033,11 +1014,12 @@ uninterned."
 (defun export (symbols &optional (package (sane-package)))
   #!+sb-doc
   "Exports SYMBOLS from PACKAGE, checking that no name conflicts result."
-  (with-packages ()
+  (with-package-graph ()
     (let ((package (find-undeleted-package-or-lose package))
+          (symbols (symbol-listify symbols))
           (syms ()))
       ;; Punt any symbols that are already external.
-      (dolist (sym (symbol-listify symbols))
+      (dolist (sym symbols)
         (multiple-value-bind (s w)
             (find-external-symbol (symbol-name sym) package)
           (declare (ignore s))
@@ -1048,8 +1030,7 @@ uninterned."
           (assert-package-unlocked package "exporting symbol~P ~{~A~^, ~}"
                                    (length syms) syms))
         ;; Find symbols and packages with conflicts.
-        (let ((used-by (package-%used-by-list package))
-              (cset ()))
+        (let ((used-by (package-%used-by-list package)))
           (dolist (sym syms)
             (let ((name (symbol-name sym)))
               (dolist (p used-by)
@@ -1059,10 +1040,7 @@ uninterned."
                              (not (member s (package-%shadowing-symbols p))))
                     ;; Beware: the name conflict is in package P, not in
                     ;; PACKAGE.
-                    (name-conflict p 'export sym sym s)
-                    (pushnew sym cset))))))
-          (when cset
-            (setq syms (set-difference syms cset))))
+                    (name-conflict p 'export sym sym s)))))))
         ;; Check that all symbols are accessible. If not, ask to import them.
         (let ((missing ())
               (imports ()))
@@ -1097,10 +1075,11 @@ uninterned."
 (defun unexport (symbols &optional (package (sane-package)))
   #!+sb-doc
   "Makes SYMBOLS no longer exported from PACKAGE."
-  (with-packages ()
+  (with-package-graph ()
     (let ((package (find-undeleted-package-or-lose package))
+          (symbols (symbol-listify symbols))
           (syms ()))
-      (dolist (sym (symbol-listify symbols))
+      (dolist (sym symbols)
         (multiple-value-bind (s w) (find-symbol (symbol-name sym) package)
           (cond ((or (not w) (not (eq s sym)))
                  (error 'simple-package-error
@@ -1126,7 +1105,7 @@ uninterned."
   "Make SYMBOLS accessible as internal symbols in PACKAGE. If a symbol is
 already accessible then it has no effect. If a name conflict would result from
 the importation, then a correctable error is signalled."
-  (with-packages ()
+  (with-package-graph ()
     (let* ((package (find-undeleted-package-or-lose package))
            (symbols (symbol-listify symbols))
            (homeless (remove-if #'symbol-package symbols))
@@ -1138,6 +1117,7 @@ the importation, then a correctable error is signalled."
                    (let ((found (member sym syms :test #'string=)))
                      (if found
                          (when (not (eq (car found) sym))
+                           (setf syms (remove (car found) syms))
                            (name-conflict package 'import sym sym (car found)))
                          (push sym syms))))
                   ((not (eq s sym))
@@ -1162,7 +1142,7 @@ the importation, then a correctable error is signalled."
   #!+sb-doc
   "Import SYMBOLS into package, disregarding any name conflict. If
   a symbol of the same name is present, then it is uninterned."
-  (with-packages ()
+  (with-package-graph ()
     (let* ((package (find-undeleted-package-or-lose package))
            (internal (package-internal-symbols package))
            (symbols (symbol-listify symbols))
@@ -1192,7 +1172,7 @@ the importation, then a correctable error is signalled."
 specified SYMBOLS. If a symbol with the given name is already present in
 PACKAGE, then the existing symbol is placed in the shadowing symbols list if
 it is not already present."
-  (with-packages ()
+  (with-package-graph ()
     (let* ((package (find-undeleted-package-or-lose package))
            (internal (package-internal-symbols package))
            (symbols (string-listify symbols))
@@ -1221,7 +1201,7 @@ it is not already present."
   "Add all the PACKAGES-TO-USE to the use list for PACKAGE so that the
 external symbols of the used packages are accessible as internal symbols in
 PACKAGE."
-  (with-packages ()
+  (with-package-graph ()
     (let ((packages (package-listify packages-to-use))
           (package (find-undeleted-package-or-lose package)))
 
@@ -1280,7 +1260,7 @@ PACKAGE."
 (defun unuse-package (packages-to-unuse &optional (package (sane-package)))
   #!+sb-doc
   "Remove PACKAGES-TO-UNUSE from the USE list for PACKAGE."
-  (with-packages ()
+  (with-package-graph ()
     (let ((package (find-undeleted-package-or-lose package))
           (packages (package-listify packages-to-unuse)))
       (with-single-package-locked-error ()
@@ -1302,12 +1282,12 @@ PACKAGE."
   "Return a list of all symbols in the system having the specified name."
   (let ((string (string string-or-symbol))
         (res ()))
-    (with-packages ()
+    (with-package-names (names)
       (maphash (lambda (k v)
                  (declare (ignore k))
                  (multiple-value-bind (s w) (find-symbol string v)
                    (when w (pushnew s res))))
-               *package-names*))
+               names))
     res))
 \f
 ;;;; APROPOS and APROPOS-LIST