Redefining a structure permanently breaks method specialized on it

[Timofei Shatrov]

Suppose there's a structure, and a method specialized on this structure.
I added a new field to this structure. This causes an error, and I choose CONTINUE restart to invalidate the existing instances of the structure. I also recompile the method, just to be safe.
However this leaves the method in a broken state. It can be called exactly once before having to be recompiled again!

Here's an example:

(defstruct test-struct abc)
(defgeneric fun-struct (struct) (:method ((s test-struct)) s))

(fun-struct (make-test-struct :abc 123)) ;; works

(defstruct test-struct abc blah) ;; invoke CONTINUE restart here

(defgeneric fun-struct (struct) (:method ((s test-struct)) s)) ;; recompiling the method

(fun-struct (make-test-struct :abc 123)) ;; works

(fun-struct (make-test-struct :abc 123)) ;; error!

debugger invoked on a SB-PCL::NO-APPLICABLE-METHOD-ERROR in thread
#<THREAD "main thread" RUNNING {1000508083}>:
  There is no applicable method for the generic function
    #<STANDARD-GENERIC-FUNCTION COMMON-LISP-USER::FUN-STRUCT (1)>
  when called with arguments
    (#S(TEST-STRUCT :ABC 123 :BLAH NIL)).

== Notes ==

1. Choosing RECKLESSLY-CONTINUE restart doesn't cause this error.
2. The method needs to be called at least once before the structure redefinition otherwise this doesn't happen.
3. Uninterning 'test-struct and recompiling the structure and method fixes this error. Otherwise it is extremely persistent (even renaming the method doesn't help!)

4. The more automated test case with HANDLER-BIND invoking CONTINUE requires redefining the structure one more time, otherwise even the first method call would fail with

debugger invoked on a SB-PCL::OBSOLETE-STRUCTURE in thread
#<THREAD "main thread" RUNNING {1000508083}>:
  obsolete structure error for a structure of type TEST-STRUCT

I don't know why the behavior is different compared to manually choosing CONTINUE restart.

(defstruct test-struct abc)
(defgeneric fun-struct (struct) (:method ((s test-struct)) s))
(fun-struct (make-test-struct :abc 123)) ;; works

(handler-bind ((simple-error (invoke-restart 'continue)))
  (defstruct test-struct abc blah))

(defstruct test-struct abc blah) ;; this is required to avoid OBSOLETE-STRUCTURE error

(defgeneric fun-struct (struct) (:method ((s test-struct)) s))

(fun-struct (make-test-struct :abc 123)) ;; works
(fun-struct (make-test-struct :abc 123)) ;; fails

== System info ==

I detected this on SBCL 2.0.0 on Windows 7 but checked the latest version on Linux just in case.

$ sbcl --version
SBCL 2.0.3

$ uname -a
Linux harime 5.1.17-x86_64-linode128 #1 SMP PREEMPT Wed Jul 10 17:11:49 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux

* *features*
(:QUICKLISP :SB-BSD-SOCKETS-ADDRINFO :ASDF3.3 :ASDF3.2 :ASDF3.1 :ASDF3 :ASDF2
 :ASDF :OS-UNIX :NON-BASE-CHARS-EXIST-P :ASDF-UNICODE :X86-64 :GENCGC :64-BIT
 :ANSI-CL :COMMON-LISP :ELF :IEEE-FLOATING-POINT :LINUX :LITTLE-ENDIAN
 :PACKAGE-LOCAL-NICKNAMES :SB-LDB :SB-PACKAGE-LOCKS :SB-THREAD :SB-UNICODE
 :SBCL :UNIX)

[Douglas Katzman (dougk)]

See http://www.lispworks.com/documentation/HyperSpec/Body/m_defstr.htm#defstruct

"The consequences of redefining a defstruct structure are undefined."

[Timofei Shatrov (timofei-shatrov)]

This is not a spec bug. It's an UI bug.

There are two restarts, CONTINUE and RECKLESSLY-CONTINUE, the first of which permanently breaks the running image in a weird way, and the second works just fine. The descriptions on the restarts are also wrong, because CONTINUE implies that everything will be fine except old instances of the structure will be obsolete. Which is a lie because it also makes methods specialized on the structure to work incorrectly, regardless of being recompiled.

[Douglas Katzman (dougk)]

It is not a UI bug. Neither restart says anything about CLOS. Neither implies that anything is "fine" on any axis of correctness with respect to instances created before the redefinition.

CONTINUE gives you a do-over on the defstruct form as long as no state was built up that hinged on the old definition. The mapping of name -> type is altered such that the type system is not intrinsically unsafe to use. This is an acceptable operation as concerns the garbage collector.

RECKLESSLY-CONTINUE *is* memory-unsafe, as recklessly continuing implies mutating the old type's metadata into the new metadata. It misleads everything in the runtime (including the garbage collector) into believing that old instances are of the new type. This is particularly unsafe, as it permits access to random words past the end of the old objects.

Basically, your example code is not a use-case. Redefining is for adding things like :type to slots (and even that is unsafe). After adding, deleting, or rearranging slots, all bets are off.

[Timofei Shatrov (timofei-shatrov)]

>with respect to instances created before the redefinition.

My use case does not involve any instances before the redefinition, nor do I expect any reasonable behavior in regards to them. At every method call a completely new structure instance is created. I'm expecting that with regards to newly created instances using a new definition of the structure and new definition of the method the behavior would be reasonable.

Consider this, even more egregious example:

(defstruct test-struct abc)
(defgeneric fun-struct (struct) (:method ((s test-struct)) s))
(fun-struct (make-test-struct :abc 123)) ;; works

(handler-bind ((simple-error (invoke-restart 'continue)))
  (defstruct test-struct abc blah))

(defstruct test-struct abc blah) ;; this is required to avoid OBSOLETE-STRUCTURE error

(defgeneric another-method (struct) (:method ((s test-struct)) s))

(another-method (make-test-struct :abc 123)) ;; works
(another-method (make-test-struct :abc 123)) ;; fails

I'm creating a brand new fresh method that is called exclusively on new instances of the structure. It's still "corrupted" by this bug by being callable exactly once, despite not even being present at the time the structure was redefined.