bug#16377: Undo Tree regression: (error "Unrecognized entry in undo list undo-tree-canary")

[Toby Cubitt <tsc25@cantab.net>]

Hi Barry,

Thanks for the bug report and recipe.

On Mon, Jan 06, 2014 at 07:32:45PM -0500, Barry OReilly wrote:
> Don't know yet if the code change belongs in Undo Tree or Emacs but
> the recipe is:
> 
>   • Start Emacs in scratch buffer with global-undo-tree-mode enabled
>   • Go to BOB and insert "xxx"
>   • Select first line
>   • undo-tree-undo in region
>   • undo-tree-redo in region
>   • undo-tree-visualize
>   • Navigate down (redo)*
>   • Navigate up (undo)
> 
> Upon the navigate up, this error occurs:
> 
>   Debugger entered--Lisp error: (error "Unrecognized entry in undo list
> undo-tree-canary")
>      signal(error ("Unrecognized entry in undo list undo-tree-canary"))
>      error("Unrecognized entry in undo list %S" undo-tree-canary)
>      primitive-undo(1 (undo-tree-canary))
>      undo-tree-undo-1(1)
>      undo-tree-visualize-undo(1)
>      call-interactively(undo-tree-visualize-undo nil nil)
>      command-execute(undo-tree-visualize-undo)

I also can't reproduce this in current stable Emacs. (Which is odd, as my
first guess from the backtrace would have been that this is a bug in the
relatively untested undo-tree undo-in-region code, and nothing to do with
Emacs.)

> * Sometimes the navigate down hangs with backtrace:
> 
>   Debugger entered--Lisp error: (quit)
>      undo-tree-copy-list((nil))
>      undo-tree-redo-1(1)
>      undo-tree-visualize-redo(1)
>      call-interactively(undo-tree-visualize-redo nil nil)
>      command-execute(undo-tree-visualize-redo)
> 
> Don't know if it's related or not.
> 
> This recipe does not cause a problem with Emacs 24.3.

Confirmed.

> The following commit is suspicious:
> 
> commit 0c180ea96c022923ef18a255d0492bdb5ad04e02
> Author: Aaron S. Hawley <aaron.s.hawley@gmail.com>
> Date:   Tue Jan 8 14:13:31 2013 -0500
> 
>     * lisp/simple.el (primitive-undo): Move from undo.c.
>     * src/undo.c (Fprimitive_undo): Move to simple.el.
>     (syms_of_undo): Remove declaration for Sprimitive_undo.
>     * test/automated/undo-tests.el: New file.
> 
> The commit moved primitive-undo from C to Elisp. In the process, it
> added an else condition not present in the C code:
>
> +             (t (error "Unrecognized entry in undo list %S" next)))))

OK, who messed with `primitive-undo' without making absolutely sure the
elisp implementation was strictly identical to the aeons-old c
implementation, and can I shoot them please? ;-)

Seriously, from the backtrace, the error does seem to be thrown by the
new test. Whether that's an Emacs regression, or just that the new test
is catching an existing bug in undo-tree, is harder to tell.

I'll play around with it in a fresh vcs Emacs when I have some time. In
the meantime, since this *is* a regression of sorts(*), can you bisect to
confirm this is the commit that broke things?

(*) I'd hope that a key requirement of moving `primitive-undo' to elisp
is surely that it not introduce changes in behaviour.

> The consequence of this bug is that when using Undo Tree, recent undo
> history can become completely inaccessible.
> 
> I should note that if I don't undo in region, Undo Tree has never run
> into any problems with the Emacs on trunk.

Have you hit other reproducible undo-in-region bugs on trunk? If so,
please report them! The undo-tree undo-in-region code is fiendishly
complex, hard to test, and under-tested.

Best,
Toby

PS: Sorry if I've broken threading on bug-gnu-emacs and generated a new
bug number. I have delivery disabled currently as I don't have time to
follow it. But it seems worth copying my reply there.
-- 
Dr T. S. Cubitt
Royal Society University Research Fellow
and Fellow of Churchill College, Cambridge
Centre for Quantum Information
DAMTP, University of Cambridge

email: tsc25@cantab.net
web:   www.dr-qubit.org