GNU bug report logs - #78980
31.0.50; C-z crashes emacsclient -nw

Previous Next

Full log

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

From: Gerd Möllmann <gerd.moellmann <at> gmail.com>
To: Pip Cet <pipcet <at> protonmail.com>
Cc: 78980 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>,
 martin rudalics <rudalics <at> gmx.at>
Subject: Re: bug#78980: 31.0.50; C-z crashes emacsclient -nw
Date: Wed, 09 Jul 2025 16:40:11 +0200

Pip Cet <pipcet <at> protonmail.com> writes:

> "Eli Zaretskii" <eliz <at> gnu.org> writes:
>
>> To reproduce:
>>
>>  $ emacs -Q -nw
>>  M-x server-start RET
>>
>> Then from another terminal:
>>
>>  $ emacsclient -nw
>>  C-z
>>
>> This crashes Emacs.  Here's the backtrace:
>
> Thanks for reporting this as a separate bug, and for taking the trouble
> to confirm it!
>
> I've been testing the possible fix for bug#78899 with this patch, but I
> don't understand the structures or how they changed lately, so all it
> does is accept that frames_with_root and frames_in_reverse_z_order can
> return nil.
>
> diff --git a/src/dispnew.c b/src/dispnew.c
> index d65a7cbc1f1..025b1fbd99a 100644
> --- a/src/dispnew.c
> +++ b/src/dispnew.c
> @@ -3455,8 +3455,8 @@ frames_in_reverse_z_order (struct frame *f, bool visible_only)
>    struct frame *root = root_frame (f);
>    Lisp_Object frames = frames_with_root (root, visible_only);
>    frames = CALLN (Fsort, frames, QClessp, Qframe__z_order_lessp);
> -  eassert (FRAMEP (XCAR (frames)));
> -  eassert (XFRAME (XCAR (frames)) == root);
> +  eassert (NILP (frames) || FRAMEP (XCAR (frames)));
> +  eassert (NILP (frames) || XFRAME (XCAR (frames)) == root);
>    return frames;
>  }
>  
> @@ -3516,7 +3516,7 @@ is_tty_root_frame_with_visible_child (struct frame *f)
>    if (!is_tty_root_frame (f))
>      return false;
>    Lisp_Object z_order = frames_in_reverse_z_order (f, true);
> -  return CONSP (XCDR (z_order));
> +  return CONSP (z_order) && CONSP (XCDR (z_order));
>  }
>  
>  /* Return the index of the first enabled row in MATRIX, or -1 if there
>
> in frames_in_reverse_z_order, the root frame is not visible, but
> visible_only is true, so frames_with_root returns Qnil correctly, I
> think.
>
> However, I don't think we should ever hit this code for invisible
> frames; maybe frame_redisplay_p should return false for them?  That
> would match its documentation:
>
> /** Return true if F can be redisplayed, that is if F is visible and, if
>     F is a tty frame, all its ancestors are visible too.  */
>
> From a76506f131b2d47b0e1dc59ecd6c581e431d298c Mon Sep 17 00:00:00 2001
> From: Pip Cet <pipcet <at> protonmail.com>
> Subject: [PATCH] Fix crashes when "emacsclient -nw" frames are suspended
>  (bug#78980)
>
> * src/frame.c (frame_redisplay_p): Start loop with 'f', not its parent
> frame.  Simplify return expression.
> ---
>  src/frame.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/src/frame.c b/src/frame.c
> index 70e200d9219..36ce36436f3 100644
> --- a/src/frame.c
> +++ b/src/frame.c
> @@ -364,8 +364,8 @@ frame_redisplay_p (struct frame *f)
>  {
>    if (is_tty_frame (f))
>      {
> -      struct frame *p = FRAME_PARENT_FRAME (f);
> -      struct frame *q = NULL;
> +      struct frame *p = f;
> +      struct frame *q = f;
>  
>        while (p)
>  	{
> @@ -387,7 +387,7 @@ frame_redisplay_p (struct frame *f)
>  	 frame of its terminal.  Any other tty frame can be redisplayed
>  	 iff it is the top frame of its terminal itself which must be
>  	 always visible.  */
> -      return (q ? q == r : f == r);
> +      return q == r;
>      }
>    else
>  #ifndef HAVE_X_WINDOWS

I haven't tested/run something, but from reading the code, I'd say the
story begins in suspend-tty, namely here

term.c<master>:
 2437       if (FRAMEP (t->display_info.tty->top_frame))
 2438         {
 2439           struct frame *top = XFRAME (t->display_info.tty->top_frame);
 2440           SET_FRAME_VISIBLE (root_frame (top), false);
 2441         }

This makes the root frame invisible, and frames_with_root, which is
called in the end, returns nil then when called with such a root and
visible_only == true. That's actually okay, I think.

I find more interesting what redisplay_internal does. The first thing
I see is

xdisp.c:
17387       /* If this is a window on a tty root frame displaying a child frame,
17388          the current matrix of W may contain glyphs of that child frame.
17389          Don't try shortcuts that might use the current matrix in this case.  */
17390       && !is_tty_root_frame_with_visible_child (XFRAME (w->frame)))

Here w->frame can be invisible, apparently. And I wonder if one should
check if the root frame of w->frame is visible as a starter. Something
like `FRAME_VISIBLE_P (root_frame (XFRAME (w->frame)))` before checking
`is_tty_root...`. Because I wonder if that wouldn't be more correct also
in the GUI case.

And there are potentially more places. I don't see at the moment if
that's already done somewhere in redisplay_internal, but maybe we should
discard redisplays for invisible frames somewhere further down, too. Or
one could first add assert that frames are visible when we display them,
e.g. in redisplay_window and so on? 

Previous Next

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