pcl method cache bug

[Kim A. Barrett <IIM@ECLA.USC.EDU>]

  While groveling through DCODE the other day, I noticed the comment in
CACHE-METHOD about not being able to depend on without-interrupts.  Since I
happen to know that without-interrupts is real for our system, I rewrote
CACHE-METHOD (and CACHED-METHOD, as well) to use it.  While I was in the
process of that rewrite, thinking about what the current code did to make sure
I wasn't missing anything cute but unobvious, I realized that the current code
is in fact wrong.  Consider the following scenarios:

1. CACHE-METHOD with one key

  Compute offset, clobber offset entry to nil, and store method-function at
offset+1.  Get interrupted.  While in the interrupt, cache a different method
with a different key which happens to hash to the same offset.  Return from the
interrupt, and install the initial key at offset.  Now a cache lookup on the
initial key will return a function which has nothing to do with the key.

2. CACHED-METHOD

  Compute offset, and successfully check all the keys for a match.  Get
interrupted.  While in the interrupt, cache a different method with a different
set of keys which happen to hash to the same offset.  Return from the
interrupt, and incorrectly return the code now in the cache.

  There are plenty of other ways to lose besides the two examples I just gave.
I actually spent some time thinking about how this could be dealt with without
using without-interrupts, but haven't come up with anything bullet-proof.
There always seems to be some critical place where either without-interrupts or
a test-and-set capability (which could be implemented using without-interrupts)
is needed.

kab

;;; The current code (in both Valentine's Day and St. Patrick's Day versions)

(defmacro cache-method (cache mask method-function &rest classes)
  `(let* ((.offset. (generic-function-cache-offset ,mask ,@classes)))
     ;; Once again, we have to endure a little brain damage because we can't
     ;; count on having without-interrupts.  I suppose the speed loss isn't
     ;; too significant since this is only when we get a cache miss.
     (setf (generic-function-cache-entry ,cache .offset. 0) nil)
     ,@(iterate ((class in (cdr classes)) (key-no from 1))
         (collect `(setf (generic-function-cache-entry ,cache
						       .offset.
						       ,key-no)
			 ,class)))
     (prog1
       (setf (generic-function-cache-entry ,cache .offset. ,(length classes))
	     ,method-function)
       (setf (generic-function-cache-entry ,cache .offset. 0)
	     ,(car classes)))))

(defmacro cached-method (cache mask &rest classes)
  `(let ((.offset. (generic-function-cache-offset ,mask . ,classes)))
     (and ,@(iterate ((class in classes) (key-no from 0))
              (collect
                `(eq (generic-function-cache-entry ,cache .offset. ,key-no)
		     ,class)))
          (generic-function-cache-entry ,cache .offset. ,(length classes)))))

-------