Subject: bug#33034: closed (Re: `unwind-protect' cleanup form is not executed if body dies in stack overflow)

From: [email-masked] (GNU bug Tracking System)
Date: Sun, 14 Oct 2018 17:04:02 +0000

Your bug report

#33034: `unwind-protect' cleanup form is not executed if body dies in stack overflow

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
33034: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=33034
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: `unwind-protect' cleanup form is not executed if body dies in stack overflow
From: Paul Eggert <[email-masked]>
To: Paul Pogonyshev <[email-masked]>
Cc: [email-masked], Eli Zaretskii <[email-masked]>
Date: Sun, 14 Oct 2018 10:02:59 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Thanks for the bug report; I installed the attached to fix it. The problem with 
your test case was neither C stack overflow nor failure to unwind the Lisp 
stack: it was failure to restore the Lisp evaluation depth (which is a separate 
thing from the Lisp stack size).

By the way, why are there two different limits? That slows the interpreter down 
a bit.  Why don't we simply have a limit for the Lisp stack size? Every time 
lisp_eval_depth grows, the stack size grows, so limiting the stack limits the 
evaluation depth for free. If we had done things this way, the interpreter would 
be a bit faster and this bug would never have occurred.
>From 5dfbdff57be6e93efe79f9b07e8b6383ec02959a Mon Sep 17 00:00:00 2001
From: Paul Eggert <[email-masked]>
Date: Sun, 14 Oct 2018 09:51:32 -0700
Subject: [PATCH] Fix lisp_eval_depth in unwind-protect cleanup

Problem reported by Paul Pogonyshev (Bug#33034).
* src/lisp.h (union specbinding): New member unwind.eval_depth.
* src/eval.c (record_unwind_protect, set_unwind_protect): Set it.
(do_one_unbind): Use it.
---
 src/eval.c | 3 +++
 src/lisp.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/src/eval.c b/src/eval.c
index 42c275de6b..a51d0c9083 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -3429,6 +3429,7 @@ record_unwind_protect (void (*function) (Lisp_Object), Lisp_Object arg)
   specpdl_ptr->unwind.kind = SPECPDL_UNWIND;
   specpdl_ptr->unwind.func = function;
   specpdl_ptr->unwind.arg = arg;
+  specpdl_ptr->unwind.eval_depth = lisp_eval_depth;
   grow_specpdl ();
 }
 
@@ -3501,6 +3502,7 @@ do_one_unbind (union specbinding *this_binding, bool unwinding,
   switch (this_binding->kind)
     {
     case SPECPDL_UNWIND:
+      lisp_eval_depth = this_binding->unwind.eval_depth;
       this_binding->unwind.func (this_binding->unwind.arg);
       break;
     case SPECPDL_UNWIND_ARRAY:
@@ -3595,6 +3597,7 @@ set_unwind_protect (ptrdiff_t count, void (*func) (Lisp_Object),
   p->unwind.kind = SPECPDL_UNWIND;
   p->unwind.func = func;
   p->unwind.arg = arg;
+  p->unwind.eval_depth = lisp_eval_depth;
 }
 
 void
diff --git a/src/lisp.h b/src/lisp.h
index 5ecc48b025..a7a26ef350 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3071,6 +3071,7 @@ union specbinding
       ENUM_BF (specbind_tag) kind : CHAR_BIT;
       void (*func) (Lisp_Object);
       Lisp_Object arg;
+      EMACS_INT eval_depth;
     } unwind;
     struct {
       ENUM_BF (specbind_tag) kind : CHAR_BIT;
-- 
2.17.1

Subject: `unwind-protect' cleanup form is not executed if body dies in stack overflow
From: Paul Pogonyshev <[email-masked]>
To: [email-masked]
Date: Sat, 13 Oct 2018 12:07:48 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

To reproduce:

    (defun overflow ()
      (overflow))
    (defun test ()
      (interactive)
      (message "BEFORE")
      (unwind-protect
          (overflow)
        (message "CLEANUP")))

Invocation of `test' never issues message "CLEANUP", whether it is run
interactively or non-interactively.

By comparison, if you _catch_ the error with `condition-case':

    (defun test-2 ()
      (interactive)
      (message "BEFORE")
      (unwind-protect
          (ignore-errors (overflow))
        (message "CLEANUP")))

then cleanup form is executed properly.

But if your error catcher is "above" the `unwind-protect' form, the
cleanup is not executed again, even though the error is eaten as
expected:

    (defun test-3 ()
      (interactive)
      (message "BEFORE")
      (ignore-errors
        (unwind-protect
            (overflow)
          (message "CLEANUP"))))

This is a perfect way to screw up your Emacs permanently (until full
restart): when some `unwind-protect' cleanups are not run, you can be
left with unexpected function advices, permanently altered global
state etc., without any good way to undestand what's wrong.

Paul