Method effectively lost after redefinition

[Siebe de Vos]

Although CLOS/MOP and concurrency may not be fully supported, the PCL code is doing some attempts to prevent race conditions. The attached example shows that this is not enough: A generic function has two methods. While one of the methods is redefined, a concurrent call to the generic function from another thread may cause the redefined method to be effectively lost forever.

This error is in my opinion much more severe than the related transient error (https://bugs.launchpad.net/sbcl/+bug/1758898).

PCL code as is uses a world lock inside defmethod and a generic function lock around changes to the dfun-state (discriminating function state). Computations of dfun-state are however not synchronized and thereby cause the reported error in race conditions.

sbcl --load method-persistent-error.lisp
This is SBCL 1.4.5, an implementation of ANSI Common Lisp.
[...]
Transient
Transient
PersistentWhile evaluating the form starting at line 48, column 0
  of #P"/path/method-persistent-error.lisp":

debugger invoked on a SIMPLE-ERROR in thread
#<THREAD "main thread" RUNNING {10005605B3}>:
  The assertion (AND (EQ (F NIL) T) (EQ (F I) I)) failed.

Type HELP for debugger help, or (SB-EXT:EXIT) to exit from SBCL.

restarts (invokable by number or by possibly-abbreviated name):
  0: [CONTINUE] Retry assertion.
  1: [RETRY ] Retry EVAL of current toplevel form.
  2: Ignore error and continue loading file "/path/method-persistent-error.lisp".
  3: [ABORT ] Abort loading file "/path/method-persistent-error.lisp".
  4: Ignore runtime option --load "/path/method-persistent-error.lisp".
  5: Skip rest of --eval and --load options.
  6: Skip to toplevel READ/EVAL/PRINT loop.
  7: [EXIT ] Exit SBCL (calling #'EXIT, killing the process).

(SB-KERNEL:ASSERT-ERROR (AND (EQ (F NIL) T) (EQ (F I) I)) NIL NIL NIL)
0]

uname -a
Linux si2l 4.4.92-31-default #1 SMP Sun Oct 22 06:56:24 UTC 2017 (1d80e8a) x86_64 x86_64 x86_64 GNU/Linux

*features* =>
(:64-BIT :64-BIT-REGISTERS :ALIEN-CALLBACKS :ANSI-CL :ASH-RIGHT-VOPS
 :C-STACK-IS-CONTROL-STACK :CALL-SYMBOL :COMMON-LISP :COMPACT-INSTANCE-HEADER
 :COMPARE-AND-SWAP-VOPS :COMPLEX-FLOAT-VOPS :CYCLE-COUNTER :ELF :FLOAT-EQL-VOPS
 :FP-AND-PC-STANDARD-SAVE :GCC-TLS :GENCGC :IEEE-FLOATING-POINT :IMMOBILE-CODE
 :IMMOBILE-SPACE :INLINE-CONSTANTS :INTEGER-EQL-VOP :LARGEFILE :LINKAGE-TABLE
 :LINUX :LITTLE-ENDIAN :MEMORY-BARRIER-VOPS :MULTIPLY-HIGH-VOPS
 :OS-PROVIDES-BLKSIZE-T :OS-PROVIDES-DLADDR :OS-PROVIDES-DLOPEN
 :OS-PROVIDES-GETPROTOBY-R :OS-PROVIDES-POLL :OS-PROVIDES-PUTWC
 :OS-PROVIDES-SUSECONDS-T :PACKAGE-LOCAL-NICKNAMES :RAW-INSTANCE-INIT-VOPS
 :RAW-SIGNED-WORD :RELOCATABLE-HEAP :SB-DOC :SB-EVAL :SB-FUTEX :SB-LDB
 :SB-PACKAGE-LOCKS :SB-SIMD-PACK :SB-SOURCE-LOCATIONS :SB-THREAD :SB-UNICODE
 :SBCL :STACK-ALLOCATABLE-CLOSURES :STACK-ALLOCATABLE-FIXED-OBJECTS
 :STACK-ALLOCATABLE-LISTS :STACK-ALLOCATABLE-VECTORS
 :STACK-GROWS-DOWNWARD-NOT-UPWARD :SYMBOL-INFO-VOPS :UNBIND-N-VOP
 :UNDEFINED-FUN-RESTARTS :UNIX :UNWIND-TO-FRAME-AND-CALL-VOP :X86-64)

[Siebe de Vos (s.de.vos)]

The attached patch extends the scope of locking to solve the issue (and https://bugs.launchpad.net/sbcl/+bug/1758898 as well). Users afraid of deadlock might want to use (setf *dfun-miss-lock-all-p* nil) to have the original behavior.