bug#48337: Fwd: 28.0.50; Emacs crashing randomly (possibly minibuffer activity related)

[Alan Mackenzie]

Hello, Eli.

On Tue, May 11, 2021 at 22:55:55 +0300, Eli Zaretskii wrote:
> > Date: Tue, 11 May 2021 19:45:23 +0000
> > Cc: Alex Bennée <alex.bennee@linaro.org>,
> >   48337@debbugs.gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > Alan, the code in nth_minibuffer and its callers is unsafe.  First,
> > > Fnthcdr can return nil, and then XCAR of that in nth_minibuffer
> > > crashes.  I fixed that now on the master branch, ....

> > That Fnthcdr call "can't possibly" return nil, unless there's a bug
> > somewhere.

> Then the commentary of nth_minibuffer is outdated and should be
> updated: it claims that returning nil is part of the contract.

I will clean up this unclean coding.

> > >   Fset_buffer (nth_minibuffer (minibuf_level));

> > This, I think, can be justified - if read_minibuf_unwind can't find
> > the minibuffer it's unwinding, we've got a serious problem and ought
> > to abort Emacs ASAP.  Should that, perhaps, be an explicit assert?

> If you want to abort, assertions is not TRT, as it will not be
> compiled in an optimized build.  Call emacs_abort instead.

Thanks for the tip!

I now understand the immediate cause of the bug completely.  Partly this
is due to me seeing better backtrace information from a subsequent post
from Alex.  This backtrace contains:

#18 0x000055e3c57335da in Frun_hooks (nargs=1, args=0x7ffe48045368) at 
eval.c:2701
......
......
#26 0x000055e3c56a82b0 in read_minibuf (map=..., initial=..., prompt=..., 
expflag=false, histvar=..., histpos=..., defalt=..., allow_props=false, 
inherit_input_method=false) at minibuf.c:683

..  The minibuf.c:683 identifies the failing point in read_minibuf as a
call to record-window-buffer.  r-w-b ends by calling the hook
buffer-list-update-hook.

At the time of calling record-window-buffer, minibuf_level has been
incremented to 2, but  *Minibuf-2* has not yet been created and added to
minibuf.c's internal list of minibuffers.  This is an inconsistent state.

Something on buffer-list-update-hook calls active-minibuffer-window, and
because of the inconsistent state, this crashes.

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

The deeper cause of the bug is that calling buffer-list-update-hook
simply doesn't belong in record-window-buffer.  That hook should be
called when the buffer list changes, not when a window's current buffer
gets "recorded".

So, as the main fix, I propose moving the call of buffer-list-update-hook
to (some of) the places where record-window-buffer gets called, those
places where the buffer list changes.  There are exactly two such places,
both in window.c.  This will prevent the chain of events in read_minibuf
outlined above.

Also, I intend to prevent the indicated inconsistency in the minibuffer
list by creating  *Minibuf-2* earlier, immediately after incrementing
minibuf_level to 2.

And, as promised, I will tidy up the untidy and unsafe coding.

Does anybody have any comments before I start hacking this?

-- 
Alan Mackenzie (Nuremberg, Germany).