GNU bug report logs - #30846
26.0.91; debug-watch of kill-all-local-variables triggers 'assertion failed: found == !EQ (blv->defcell, blv->valcell)'

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 30846 in the body.
You can then email your comments to 30846 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Sun, 18 Mar 2018 09:10:41 -0400

[Message part 1 (text/plain, inline)]

Evaluate the following from 'emacs -Q':

    (setq-local foo 1)

    ;; Simulate (debug-watch 'foo) + continue from *Backtrace*
    (add-variable-watcher 'foo (lambda (symbol newval operation where)
                                 (with-temp-buffer
                                   (kill-all-local-variables))))
    (fundamental-mode)

This results in

../../src/data.c:98: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell)

Backtrace attached.  I guess it has something to do with the recursive
`kill-all-local-variables' call, although I'm not familiar enough with
the local variable machinery to say more about it.

[assert-fail-backtrace.log (text/plain, attachment)]

[Message part 3 (text/plain, inline)]

In GNU Emacs 26.0.91 (build 54, x86_64-pc-linux-gnu, X toolkit, Xaw scroll bars)
 of 2018-03-18 built on zebian
Repository revision: 10bd3b3af8acfc226acadc654298865cffc19cc9

Configured using:
 'configure --cache-file=../../debug-config.cache 'CC=ccache gcc'
 'CFLAGS=-O0 -g3 -march=native' --enable-checking=yes
 --enable-check-lisp-object-type --with-x-toolkit=lucid
 --with-toolkit-scroll-bars --with-gif=no --with-jpeg=no'

Message #8 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Sun, 18 Mar 2018 16:05:37 +0200

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sun, 18 Mar 2018 09:10:41 -0400
> 
> Evaluate the following from 'emacs -Q':
> 
>     (setq-local foo 1)
> 
>     ;; Simulate (debug-watch 'foo) + continue from *Backtrace*
>     (add-variable-watcher 'foo (lambda (symbol newval operation where)
>                                  (with-temp-buffer
>                                    (kill-all-local-variables))))
>     (fundamental-mode)
> 
> This results in
> 
> ../../src/data.c:98: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell)
> 
> Backtrace attached.  I guess it has something to do with the recursive
> `kill-all-local-variables' call, although I'm not familiar enough with
> the local variable machinery to say more about it.

Do you mean that the inner call to kill-all-local-variables steps on
toes of the outer call, and thus corrupts the local values or
something?  If so, do you see any signs of such a corruption?

Message #11 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Sun, 18 Mar 2018 16:07:33 +0200

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Sun, 18 Mar 2018 09:10:41 -0400
> 
> Evaluate the following from 'emacs -Q':
> 
>     (setq-local foo 1)
> 
>     ;; Simulate (debug-watch 'foo) + continue from *Backtrace*
>     (add-variable-watcher 'foo (lambda (symbol newval operation where)
>                                  (with-temp-buffer
>                                    (kill-all-local-variables))))
>     (fundamental-mode)
> 
> This results in
> 
> ../../src/data.c:98: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell)
> 
> Backtrace attached.  I guess it has something to do with the recursive
> `kill-all-local-variables' call, although I'm not familiar enough with
> the local variable machinery to say more about it.

Do you mean that the inner call to kill-all-local-variables steps on
toes of the outer call, and thus corrupts the local values or
something?  If so, do you see any signs of such a corruption?  Because
otherwise maybe the assertion is wrong?

Message #14 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Sun, 18 Mar 2018 10:20:54 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Date: Sun, 18 Mar 2018 09:10:41 -0400
>> 
>> Evaluate the following from 'emacs -Q':
>> 
>>     (setq-local foo 1)
>> 
>>     ;; Simulate (debug-watch 'foo) + continue from *Backtrace*
>>     (add-variable-watcher 'foo (lambda (symbol newval operation where)
>>                                  (with-temp-buffer
>>                                    (kill-all-local-variables))))
>>     (fundamental-mode)
>> 
>> This results in
>> 
>> ../../src/data.c:98: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell)
>> 
>> Backtrace attached.  I guess it has something to do with the recursive
>> `kill-all-local-variables' call, although I'm not familiar enough with
>> the local variable machinery to say more about it.
>
> Do you mean that the inner call to kill-all-local-variables steps on
> toes of the outer call, and thus corrupts the local values or
> something?  If so, do you see any signs of such a corruption?  Because
> otherwise maybe the assertion is wrong?

No, I haven't seen any signs of corrupted values, though I'm not sure
exactly where to look.  It's possible the assertion is wrong (or more
precisely, that the variable watcher breaks the assertion without
breaking anything else).  I don't really understand what the assertion
is testing, at a high level (that is, why does it expect 'defcell' and
'valcell' to have that relation).

Message #17 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Sun, 18 Mar 2018 16:38:13 +0200

> From: Noam Postavsky <npostavs <at> gmail.com>
> Cc: 30846 <at> debbugs.gnu.org
> Date: Sun, 18 Mar 2018 10:20:54 -0400
> 
> I don't really understand what the assertion is testing, at a high
> level (that is, why does it expect 'defcell' and 'valcell' to have
> that relation).

Yes, the 'found' member could use a better documentation.  (Btw, all
calls to set_blv_found seem to use the last argument of zero.)

I hope Stefan can comment on this.

Message #20 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Stefan Monnier <monnier <at> iro.umontreal.ca>, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Tue, 20 Mar 2018 20:48:50 -0400

Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Noam Postavsky <npostavs <at> gmail.com>
>> Cc: 30846 <at> debbugs.gnu.org
>> Date: Sun, 18 Mar 2018 10:20:54 -0400
>> 
>> I don't really understand what the assertion is testing, at a high
>> level (that is, why does it expect 'defcell' and 'valcell' to have
>> that relation).
>
> Yes, the 'found' member could use a better documentation.  (Btw, all
> calls to set_blv_found seem to use the last argument of zero.)
>
> I hope Stefan can comment on this.

I'll add him to Cc then, I think he's not subscribed to the bug list.

Message #23 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Wed, 21 Mar 2018 09:04:09 -0400

> I'll add him to Cc then, I think he's not subscribed to the bug list.

Indeed, I'm not, thanks.

I think there's a real bug in there (kill-local-variable may leave
valcell pointing to the old (now used) cell), let me take a closer look.


        Stefan

Message #26 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Thu, 22 Mar 2018 11:45:32 -0400

> This results in
>
> ../../src/data.c:98: Emacs fatal error: assertion failed: found == !EQ (blv->defcell, blv->valcell)

OK, I found the culprit: in kill-all-local-variables, we first change
all the (relevant) symbols to point to their global value (with
swap_in_global_binding called from swap_out_buffer_local_variables),
then go and kill them one by one (in reset_buffer_local_variables).
This worked fine in the past, but now that we run watchers while we kill
the vars, the act of running the watchers may undo the effect of
swap_in_global_binding, so we can't kill them quite in the same way.

The patch below against emacs-26 seems to fix the problem (it mostly
merges the code of swap_out_buffer_local_variables into that of
reset_buffer_local_variables so that the swap_in_global_binding is done
just before we actually kill the var, with no opportunity for watchers
to undo the effect).

The patch doesn't only fix this problem, it also changes the time at
which we run the watcher: in the current emacs-26 code,
kill-all-local-variables runs the watcher *after* killing the
corresponding var, whereas usually the watchers are run *before* the var
is modified.  I can split the patch into two, if you want and/or only
apply the part that actually fixes this bug.

If you feel this is too risky for emacs-26, I wouldn't blame you (this
is pretty tricky code): while the assertion crashes Emacs, a normal
build without assertions will likely not notice the problem at all.
I came up with a test case that catches the problem, but I think that in
"real" life it's very unlikely to cause a problem.

It applies unchanged to master (and while looking at this bug I found
a whole bunch of other minor changes that I plan to install into master
anyway).


        Stefan


diff --git a/src/buffer.c b/src/buffer.c
index 9b54e4b778..b0cee71703 100644
--- a/src/buffer.c
+++ b/src/buffer.c
@@ -108,7 +108,6 @@ int last_per_buffer_idx;
 static void call_overlay_mod_hooks (Lisp_Object list, Lisp_Object overlay,
                                     bool after, Lisp_Object arg1,
                                     Lisp_Object arg2, Lisp_Object arg3);
-static void swap_out_buffer_local_variables (struct buffer *b);
 static void reset_buffer_local_variables (struct buffer *, bool);
 
 /* Alist of all buffer names vs the buffers.  This used to be
@@ -991,10 +990,29 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
   else
     {
       Lisp_Object tmp, last = Qnil;
+      Lisp_Object buffer;
+      XSETBUFFER (buffer, b);
+
       for (tmp = BVAR (b, local_var_alist); CONSP (tmp); tmp = XCDR (tmp))
         {
           Lisp_Object local_var = XCAR (XCAR (tmp));
           Lisp_Object prop = Fget (local_var, Qpermanent_local);
+          Lisp_Object sym = local_var;
+
+          /* Watchers are run *before* modifying the var.  */
+          if (XSYMBOL (local_var)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
+            notify_variable_watchers (local_var, Qnil,
+                                      Qmakunbound, Fcurrent_buffer ());
+
+          eassert (XSYMBOL (sym)->u.s.redirect == SYMBOL_LOCALIZED);
+          /* Need not do anything if some other buffer's binding is
+	     now cached.  */
+          if (EQ (SYMBOL_BLV (XSYMBOL (sym))->where, buffer))
+	    {
+	      /* Symbol is set up for this buffer's old local value:
+	         swap it out!  */
+	      swap_in_global_binding (XSYMBOL (sym));
+	    }
 
           if (!NILP (prop))
             {
@@ -1034,10 +1052,6 @@ reset_buffer_local_variables (struct buffer *b, bool permanent_too)
             bset_local_var_alist (b, XCDR (tmp));
           else
             XSETCDR (last, XCDR (tmp));
-
-          if (XSYMBOL (local_var)->u.s.trapped_write == SYMBOL_TRAPPED_WRITE)
-            notify_variable_watchers (local_var, Qnil,
-                                      Qmakunbound, Fcurrent_buffer ());
         }
     }
 
@@ -1867,7 +1881,6 @@ cleaning up all windows currently displaying the buffer to be killed. */)
      won't be protected from GC.  They would be protected
      if they happened to remain cached in their symbols.
      This gets rid of them for certain.  */
-  swap_out_buffer_local_variables (b);
   reset_buffer_local_variables (b, 1);
 
   bset_name (b, Qnil);
@@ -2737,11 +2750,6 @@ the normal hook `change-major-mode-hook'.  */)
 {
   run_hook (Qchange_major_mode_hook);
 
-  /* Make sure none of the bindings in local_var_alist
-     remain swapped in, in their symbols.  */
-
-  swap_out_buffer_local_variables (current_buffer);
-
   /* Actually eliminate all local bindings of this buffer.  */
 
   reset_buffer_local_variables (current_buffer, 0);
@@ -2753,31 +2761,6 @@ the normal hook `change-major-mode-hook'.  */)
   return Qnil;
 }
 
-/* Make sure no local variables remain set up with buffer B
-   for their current values.  */
-
-static void
-swap_out_buffer_local_variables (struct buffer *b)
-{
-  Lisp_Object oalist, alist, buffer;
-
-  XSETBUFFER (buffer, b);
-  oalist = BVAR (b, local_var_alist);
-
-  for (alist = oalist; CONSP (alist); alist = XCDR (alist))
-    {
-      Lisp_Object sym = XCAR (XCAR (alist));
-      eassert (XSYMBOL (sym)->u.s.redirect == SYMBOL_LOCALIZED);
-      /* Need not do anything if some other buffer's binding is
-	 now cached.  */
-      if (EQ (SYMBOL_BLV (XSYMBOL (sym))->where, buffer))
-	{
-	  /* Symbol is set up for this buffer's old local value:
-	     swap it out!  */
-	  swap_in_global_binding (XSYMBOL (sym));
-	}
-    }
-}
 
 /* Find all the overlays in the current buffer that contain position POS.
    Return the number found, and store them in a vector in *VEC_PTR.
diff --git a/test/src/data-tests.el b/test/src/data-tests.el
index dda1278b6d..34637e4bfb 100644
--- a/test/src/data-tests.el
+++ b/test/src/data-tests.el
@@ -1,4 +1,4 @@
-;;; data-tests.el --- tests for src/data.c
+;;; data-tests.el --- tests for src/data.c  -*- lexical-binding:t -*-
 
 ;; Copyright (C) 2013-2018 Free Software Foundation, Inc.
 
@@ -484,3 +484,20 @@ binding-test-some-local
       (remove-variable-watcher 'data-tests-lvar collect-watch-data)
       (setq data-tests-lvar 6)
       (should (null watch-data)))))
+
+(ert-deftest data-tests-kill-all-local-variables () ;bug#30846
+  (with-temp-buffer
+    (setq-local data-tests-foo1 1)
+    (setq-local data-tests-foo2 2)
+    (setq-local data-tests-foo3 3)
+    (let ((oldfoo2 nil))
+      (add-variable-watcher 'data-tests-foo2
+                            (lambda (&rest _)
+                              (setq oldfoo2 (bound-and-true-p data-tests-foo2))))
+      (kill-all-local-variables)
+      (should (equal oldfoo2 '2)) ;Watcher is run before changing the var.
+      (should (not (or (bound-and-true-p data-tests-foo1)
+                       (bound-and-true-p data-tests-foo2)
+                       (bound-and-true-p data-tests-foo3)))))))
+
+;;; data-tests.el ends here

Message #29 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: npostavs <at> gmail.com, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Thu, 22 Mar 2018 17:53:22 +0200

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Date: Thu, 22 Mar 2018 11:45:32 -0400
> Cc: 30846 <at> debbugs.gnu.org
> 
> If you feel this is too risky for emacs-26, I wouldn't blame you (this
> is pretty tricky code): while the assertion crashes Emacs, a normal
> build without assertions will likely not notice the problem at all.
> I came up with a test case that catches the problem, but I think that in
> "real" life it's very unlikely to cause a problem.
> 
> It applies unchanged to master (and while looking at this bug I found
> a whole bunch of other minor changes that I plan to install into master
> anyway).

I indeed prefer to install this on master, not on the release branch.

Thanks.

Message #32 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Thu, 22 Mar 2018 20:20:13 -0400

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> If you feel this is too risky for emacs-26, I wouldn't blame you (this
> is pretty tricky code): while the assertion crashes Emacs, a normal
> build without assertions will likely not notice the problem at all.
> I came up with a test case that catches the problem, but I think that in
> "real" life it's very unlikely to cause a problem.

Should we disable the assertion in emacs-26 then?

And would the diff below updating comments on struct
Lisp_Buffer_Local_Value be correct?

--- i/src/lisp.h
+++ w/src/lisp.h
@@ -2593,10 +2593,10 @@ XUSER_PTR (Lisp_Object a)
    variable, you must first make sure the right binding is loaded;
    then you can access the value in (or through) `realvalue'.
 
-   `buffer' and `frame' are the buffer and frame for which the loaded
-   binding was found.  If those have changed, to make sure the right
+   `where' is the buffer for which the loaded
+   binding was found.  If it has changed, to make sure the right
    binding is loaded it is necessary to find which binding goes with
-   the current buffer and selected frame, then load it.  To load it,
+   the current buffer, then load it.  To load it,
    first unload the previous binding, then copy the value of the new
    binding into `realvalue' (or through it).  Also update
    LOADED-BINDING to point to the newly loaded binding.
@@ -2615,14 +2615,14 @@ XUSER_PTR (Lisp_Object a)
     bool_bf found : 1;
     /* If non-NULL, a forwarding to the C var where it should also be set.  */
     union Lisp_Fwd *fwd;	/* Should never be (Buffer|Kboard)_Objfwd.  */
-    /* The buffer or frame for which the loaded binding was found.  */
+    /* The buffer for which the loaded binding was found.  */
     Lisp_Object where;
     /* A cons cell that holds the default value.  It has the form
        (SYMBOL . DEFAULT-VALUE).  */
     Lisp_Object defcell;
     /* The cons cell from `where's parameter alist.
        It always has the form (SYMBOL . VALUE)
-       Note that if `forward' is non-nil, VALUE may be out of date.
+       Note that if `fwd' is non-NULL, VALUE may be out of date.
        Also if the currently loaded binding is the default binding, then
        this is `eq'ual to defcell.  */
     Lisp_Object valcell;

Message #35 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Thu, 22 Mar 2018 21:22:19 -0400

> Should we disable the assertion in emacs-26 then?

I don't have an opinion on this.

> And would the diff below updating comments on struct
> Lisp_Buffer_Local_Value be correct?

Yes.


        Stefan

Message #38 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Noam Postavsky <npostavs <at> gmail.com>
Cc: monnier <at> IRO.UMontreal.CA, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 11:12:18 +0300

> From: Noam Postavsky <npostavs <at> gmail.com>
> Date: Thu, 22 Mar 2018 20:20:13 -0400
> Cc: 30846 <at> debbugs.gnu.org
> 
> Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:
> 
> > If you feel this is too risky for emacs-26, I wouldn't blame you (this
> > is pretty tricky code): while the assertion crashes Emacs, a normal
> > build without assertions will likely not notice the problem at all.
> > I came up with a test case that catches the problem, but I think that in
> > "real" life it's very unlikely to cause a problem.
> 
> Should we disable the assertion in emacs-26 then?

Do we believe people build a production version with --enable-checking?

Message #41 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Noam Postavsky <npostavs <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 08:25:19 -0400

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

>> And would the diff below updating comments on struct
>> Lisp_Buffer_Local_Value be correct?
>
> Yes.

Pushed to emacs-26 [1: b8ebf5fb64].

Eli Zaretskii <eliz <at> gnu.org> writes:

>> Should we disable the assertion in emacs-26 then?

> Do we believe people build a production version with --enable-checking?

Hmm, no, probably not.

[1: b8ebf5fb64]: 2018-03-23 08:19:42 -0400
  * src/lisp.h (struct Lisp_Buffer_Local_Value): Update commentary.
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=b8ebf5fb64dbf261315bfdb281a8b0a119e7cc2b

Message #44 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Noam Postavsky <npostavs <at> gmail.com>, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 08:57:13 -0400

>> > If you feel this is too risky for emacs-26, I wouldn't blame you (this
>> > is pretty tricky code): while the assertion crashes Emacs, a normal
>> > build without assertions will likely not notice the problem at all.
>> > I came up with a test case that catches the problem, but I think that in
>> > "real" life it's very unlikely to cause a problem.
>> Should we disable the assertion in emacs-26 then?
> Do we believe people build a production version with --enable-checking?

Does that mean you'd rather not install my patch into emacs-26 or are
you still undecided?


        Stefan

Message #47 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: npostavs <at> gmail.com, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 17:23:14 +0300

> From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
> Cc: Noam Postavsky <npostavs <at> gmail.com>, 30846 <at> debbugs.gnu.org
> Date: Fri, 23 Mar 2018 08:57:13 -0400
> 
> >> > If you feel this is too risky for emacs-26, I wouldn't blame you (this
> >> > is pretty tricky code): while the assertion crashes Emacs, a normal
> >> > build without assertions will likely not notice the problem at all.
> >> > I came up with a test case that catches the problem, but I think that in
> >> > "real" life it's very unlikely to cause a problem.
> >> Should we disable the assertion in emacs-26 then?
> > Do we believe people build a production version with --enable-checking?
> 
> Does that mean you'd rather not install my patch into emacs-26 or are
> you still undecided?

I'm quite sure we shouldn't put this on emacs-26, I was just replying
to Noam's suggestion to remove the assertion.

Message #50 received at 30846 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: npostavs <at> gmail.com, 30846 <at> debbugs.gnu.org
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 10:42:00 -0400

>> Does that mean you'd rather not install my patch into emacs-26 or are
>> you still undecided?
> I'm quite sure we shouldn't put this on emacs-26, I was just replying
> to Noam's suggestion to remove the assertion.

OK, thanks, I'll prepare my patch for master, then.


        Stefan

Message #55 received at 30846-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: 30846-done <at> debbugs.gnu.org
Cc: Noam Postavsky <npostavs <at> gmail.com>
Subject: Re: bug#30846: 26.0.91;
 debug-watch of kill-all-local-variables triggers 'assertion failed:
 found == !EQ (blv->defcell, blv->valcell)'
Date: Fri, 23 Mar 2018 11:29:55 -0400

Version: 27.1

> The patch doesn't only fix this problem, it also changes the time at
> which we run the watcher: in the current emacs-26 code,

Installed into master,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.