Subject: Re: bug#24640: Crashes in 25.1

From: Toby Cubitt <[email-masked]>
Date: Wed, 12 Oct 2016 17:56:56 +0100

On Wed, Oct 12, 2016 at 05:44:55PM +0300, Eli Zaretskii wrote:
> What's more, the steps to reproduce may be simple (start Emacs and
> wait for it to crash), finding the code that is the culprit for this
> is anything but easy.  The crash is triggered by GC, and is due to an
> invalid object being present in the value of read_objects, which holds
> the object read by Emacs with the #n=object form.  The problem is that
> the faulty object is deep in that list, so catching the code that puts
> it there is not easy.  I'm still trying to devise a way for that, with
> no real success so far.

I understand. I can't help at all with the GC bug, unfortunately, as it's
well outside my Emacs knowledge.

I know what the buffer-undo-tree data structure looks like that gets
written and read from file, so if I can help at any point by picking
apart or simplifying that lisp structure, let me know.

> > > Some functions in undo-tree refer to or manipulate Emacs undo
> > > internals:
> > > 
> > >   undo-list-pop-changeset
> > >   undo-list-transfer-to-tree
> > >   undo-list-rebuild-from-tree
> > >   undo-tree-pull-undo-in-region-branch
> > >   undo-tree-pull-redo-in-region-branch
> > >   undo-tree-adjust-elements-to-elt
> > >   undo-tree-apply-deltas
> > >   undo-tree-undo-1
> > >   undo-tree-redo-1
> > > 
> > > Do they perhaps need some adjustments to Emacs 25's undo?
> > 
> > The only Emacs undo internal that undo-tree manipulates is the
> > buffer-undo-list variable. The only functions that do so are:
> > 
> >   undo-list-pop-changeset
> >   undo-list-transfer-to-tree
> >   undo-list-rebuild-from-tree
> > 
> > All the others either call one of these, or only touch buffer-undo-tree
> > which is a variable defined in the undo-tree package and which the Emacs
> > undo internals know nothing about.
> > 
> > Has the format of buffer-undo-list has changed at all? If so, then the
> > three above might need adjustment. If not, they should work as before.
> > 
> > The only other Emacs undo internals used by undo-tree are to call
> > primitive-undo and undo-boundary when undo/redoing.
> 
> I counted this last class among those listed above.

Indeed. I was attempting to explain which ones manipulate
buffer-undo-list "behind Emacs' back", and which ones just call out to
undo functions.

In case it helps, note that none of the above functions get called when
reloading history from file.

> > > Another potential issue is the new undo timer we have in Emacs 25 (see
> > > undo-auto--boundary-ensure-timer in simple.el).  One way of checking
> > > whether this is related to the crashes is to modify that function to
> > > use a much larger value for the 1st argument of run-at-time, say
> > > 10000, so that the undo timer never fires during the startup.  Reuben,
> > > could you try that?
> > 
> > As far as I understand, the timer just adds undo boundaries to
> > buffer-undo-list in some of the open buffers. Undo-tree doesn't touch
> > buffer-undo-list (or do anything at all, in fact) until you call one of
> > its interactive commands.
> 
> Does restoring undo-tree history manipulates buffer-undo-list of any
> buffers in any way?

No. It just reads a lisp structure from file into the buffer-undo-tree
variable.

> > Since the timer can't run whilst undo-tree lisp code is running
> 
> It can run if during restoring the history, undo-tree calls sit-for, or
> asks a question, or calls some other API that enters redisplay and/or
> involves user input.

During history loading it doesn't access buffer-undo-list at all, so it
shouldn't matter if the timer runs.

When undoing, everything that accesses or touches buffer-undo-list is
encapsulated in the three functions I listed. None of these call sit-for,
prompt for input, or display anything. As far as I understand it, they
shouldn't be able to trigger redisplay at all (caveat I'm no redisplay
expert - that's you!)

I don't think the undo timer can trigger elsewhere during undo-tree
undo. Even if it does somehow trigger outside those three functions, this
shouldn't break anything. Depending on when it triggers, undo-tree will
either pick up the extra undo boundary added to buffer-undo-list this
time around, or next time you undo.

> > I'd be surprised if the new undo timer is the culprit.
> 
> It could be a culprit, but evidently isn't, as Reuben already tried to
> disable it.

Indeed. But even if it's not to blame here, I still ought double-check
the above carefully to make sure the new undo timer doesn't interact with
undo-tree in some subtle way I've overlooked.

Cheers,
Toby
-- 
Dr T. S. Cubitt
Royal Society University Research Fellow
Quantum Information Theory
Department of Computer Science
University College London

email: [email-masked]
web:   www.dr-qubit.org