armedbear / abcl

Armed Bear Common Lisp <git+https://github.com/armedbear/abcl/> <--> <svn+https://abcl.org/svn> Bridge
https://abcl.org#rdfs:seeAlso<https://gitlab.common-lisp.net/abcl/abcl>
Other
290 stars 29 forks source link

UNWIND-PROTECT cleanup forms are not evaluated on thread destruction #430

Open sionescu opened 2 years ago

sionescu commented 2 years ago

It looks like THREADS:DESTROY-THREAD does not evaluate cleanup forms, but simply terminates the thread.

Test case:

(defparameter *test* nil)

(defun thread-termination-test ()
  (setf *test* nil)
  (flet ((thread-fn ()
           (setf *test* :entered)
           (unwind-protect
               (progn
                 ;; Ensure that the thread is blocked here
                 ;; when DESTROY-THREAD is called.
                 (sleep 3)
                 (setf *test* :failed))
             (when (eq *test* :entered)
               (setf *test* :success)))))
    (let ((thread (threads:make-thread #'thread-fn)))
      (sleep 1)
      (threads:destroy-thread thread)
      (threads:thread-join thread)
      *test*)))

On all Lisps that I can get my hands on, the equivalent code in Bordeaux-Threads returns :SUCCESS, while ABCL returns :ENTERED.

phoe commented 2 years ago

Weird.

https://github.com/armedbear/abcl/blob/36a4b5994227d768882ff6458b3df9f79caac664/src/org/armedbear/lisp/Lisp.java#L515-L516

If a thread is marked to be destroyed, then a Java error (not an exception) is eventually thrown. That should unwind the stack and therefore should execute unwind-protect cleanup forms.

Are these defined to behave like Java finally? Unwinding the Java stack should cause the finally forms between the Java throw and the Java catch to be executed.

phoe commented 2 years ago

A solution that should always work, but that would change the architecture of destroying threads, would be to implement destroying a thread via interrupting it with (lambda () (throw '%abcl-destroy-thread nil)) after wrapping all thread code in (catch '%abcl-destroy-thread ...) - if CL throws and catches work fine, then this should also work fine.