Steel Bank Common Lisp

FIND-RESTART, when given a RESTART, omits to check if the restart is still active according to its test-function and condition-restarts associations

Reported by Jean-Philippe Paradis on 2011-04-30
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
SBCL
Undecided
Unassigned

Bug Description

What I do:
(let ((activep t))
  (restart-bind ((dynamically-active-restart
                  (constantly 'irrelevant)
                  :test-function (lambda (condition)
                                   (declare (ignore condition))
                                   activep)))
    (let ((actual-restart
           (find-restart 'dynamically-active-restart)))
      (values
        ;; Inactive because of condition-restarts associations.
        (let ((required-condition (make-condition 'condition))
              (wrong-condition (make-condition 'condition)))
          (with-condition-restarts required-condition (list actual-restart)
            (find-restart actual-restart wrong-condition)))
        ;; Inactive because of test-function.
        (progn
          (setf activep nil)
          (find-restart actual-restart))))))

What happens:
Values returned:
#<RESTART DYNAMICALLY-ACTIVE-RESTART {B8500C9}>
#<RESTART DYNAMICALLY-ACTIVE-RESTART {B8500C9}>

What I expected to happen:
Values returned:
NIL
NIL

Analysis:
From CLHS FIND-RESTART:

"If identifier is a currently active restart, then it is returned. Otherwise, nil is returned."

I'm pretty sure a restart is not considered "active" if its test-function returns false.
I'm less sure about condition-restarts associations, but I think the same behavior (returning NIL) should apply.

The only way to test if a RESTART that you have in your hands is still valid is to call FIND-RESTART on it, since there's no portable way to query condition-restarts associations or call a RESTART's test-function directly. It might be possible to test the restart for validity by trying to FIND-RESTART its RESTART-NAME, but this is inefficient, unidiomatic, error-prone (this restart might have been shadowed by another restart of the same name since FIND-RESTART was called the first time), and convoluted compared to just using the restart itself.

Note that INVOKE-RESTART has the same type of problem (calling the restart-function when given an actual restart as argument while still in its dynamic extent yet being inactive because of condition-restarts associations or test-function returning false), but in SBCL INVOKE-RESTART indirectly invokes FIND-RESTART so this is really the same problem...

From CLHS INVOKE-RESTART:
"If restart is not valid, an error of type control-error is signaled."

SBCL version: 1.0.42
uname -a: Linux dynamorph 2.6.32-30-generic #59-Ubuntu SMP Tue Mar 1 21:30:21 UTC 2011 i686 GNU/Linux

*features*:
(:SWANK :QUICKLISP :SB-BSD-SOCKETS-ADDRINFO :ASDF2 :ASDF :ANSI-CL :COMMON-LISP
 :SBCL :SB-DOC :SB-TEST :SB-LDB :SB-PACKAGE-LOCKS :SB-UNICODE :SB-EVAL
 :SB-SOURCE-LOCATIONS :IEEE-FLOATING-POINT :X86 :UNIX :ELF :LINUX :SB-THREAD
 :LARGEFILE :GENCGC :STACK-GROWS-DOWNWARD-NOT-UPWARD :C-STACK-IS-CONTROL-STACK
 :COMPARE-AND-SWAP-VOPS :UNWIND-TO-FRAME-AND-CALL-VOP :RAW-INSTANCE-INIT-VOPS
 :STACK-ALLOCATABLE-CLOSURES :STACK-ALLOCATABLE-VECTORS
 :STACK-ALLOCATABLE-LISTS :STACK-ALLOCATABLE-FIXED-OBJECTS :ALIEN-CALLBACKS
 :CYCLE-COUNTER :INLINE-CONSTANTS :MEMORY-BARRIER-VOPS :LINKAGE-TABLE
 :OS-PROVIDES-DLOPEN :OS-PROVIDES-PUTWC :OS-PROVIDES-SUSECONDS-T)

Jan Moringen (scymtym) wrote :

Other implementations seem to support the interpretation in the report (see results below).

See https://bugs.launchpad.net/sbcl/+bug/769615 for a patch which adds MAP-RESTARTS and also changes the behavior of FIND-RESTART as requested here.

restart.lisp:
(let ((activep t))
  (restart-bind ((dynamically-active-restart
                  (constantly 'irrelevant)
                  :test-function (lambda (condition)
                                   (declare (ignore condition))
                                   activep)))
    (let ((actual-restart
           (find-restart 'dynamically-active-restart)))
      ;; Inactive because of condition-restarts associations.
      (let ((required-condition (make-condition 'condition))
            (wrong-condition (make-condition 'condition)))
        (with-condition-restarts required-condition (list actual-restart)
          (print (find-restart actual-restart wrong-condition))))
        ;; Inactive because of test-function.
        (setf activep nil)
        (print (find-restart actual-restart)))))

ECL
(Embeddable Common-Lisp) 12.12.1
(git:e725846659f584e5ad048d360e955f6512402eee)

$ ~/opt/ecl/bin/ecl -load restart.lisp
;;; Loading "/tmp/restart.lisp"

NIL
NIL

CLISP
GNU CLISP 2.49 (2010-07-07) (built on rothera.buildd [10.211.37.14])

$ clisp restart.lisp

NIL
NIL

CCL
Clozure Common Lisp Version 1.8-r15286M (LinuxX8632)!

$ ~/opt/ccl/lx86cl -l restart.lisp

NIL
NIL

Changed in sbcl:
status: New → Fix Committed
Changed in sbcl:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers