Subject: Re: bug#24640: Crashes in 25.1

From: Eli Zaretskii <[email-masked]>
Date: Wed, 12 Oct 2016 17:44:55 +0300

> Date: Wed, 12 Oct 2016 14:50:20 +0100
> Cc: [email-masked], [email-masked], [email-masked]
> From: Toby Cubitt <[email-masked]>
> 
> On Wed, Oct 12, 2016 at 01:31:05PM +0300, Eli Zaretskii wrote:
> > > Date: Tue, 11 Oct 2016 19:33:20 +0300
> > > From: Eli Zaretskii <[email-masked]>
> > > Cc: [email-masked]
> > > 
> > > > ​Yes, the crashes appear to stop when I comment out (global-undo-tree-mode) in vars.el.
> 
> You can also disable history loading without disabling
> global-undo-tree-mode, by disabling undo-tree-auto-save-history. Might be
> worth doing this more fine-grained check to confirm.

Thanks for chiming in.

> > > Phillip, could you please look into that package and see if you can
> > > spot any potential problems with the Emacs 25 undo internals?  TIA.
> 
> >From the bug tracker discussion, it sounds like you're close to boiling
> this down to some simple steps involving one undo-tree history file that
> trigger the crash.

They are only simple on Reuben's machine, as they involve a very large
setup of his Emacs sessions.  We don't have a reproducer for any other
machine.

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.

> If you can send me the file and a list of steps to reproduce from
> emacs -Q, I'd be happy to look into the undo-tree side of things.

Thanks.  Maybe Reuben will be able to do that.

> > 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.

> > 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?

> 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.

> 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.

Thanks.