Subject: Re: bug#75056: 31.0.50; tty-child-frames with server / multiple clients possible hangs

From: Gerd Möllmann <[email-masked]>
Date: Wed, 29 Jan 2025 08:48:56 +0100

Gerd Möllmann <[email-masked]> writes:

>>> (make-frame '((window-system . x)))
>>>
>>> move to the graphical and back to the tty frame	I get a crash like
>>>
>>> #0  terminate_due_to_signal (sig=6, backtrace_limit=2147483647) at ../../src/emacs.c:432
>
> FWIW, I tried the equivalent with (window-system . ns) here, and
> couldn't provoke the assertion.

Using the right worktree helped.

Taking a closer look at tty_set_cursor, using the selected frame here
doesn't make sense to me. I don't see a good reason why the selected
frame has to have anything to do with where to place the cursor in an
updated frame. That looks more like a left-over from times before
multi-tty.

Could you please try the attached, Martin? The first one is what I think
it the fix. The second one is a cleanup that leads to more checks
without having GLYPH_DEBUG. If this also work for you, I'll put that in
master.

(I see a "flicker" of the tty frame after the make-frame. That is also
the case in Emacs 30.)

>From 105f818c4c1479f1ef60b9d0cdb2b4375021f3ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Wed, 29 Jan 2025 08:28:58 +0100
Subject: [PATCH 1/2] Don't use selected frame in tty_set_cursor

* src/dispnew.c (tty_set_cursor): Add frmae parameter.
(write_matrix): Pass to the updated frame to tty_set_cursor.
---
 src/dispnew.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 1b4c5eb37cf..4b09282121a 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -5618,10 +5618,8 @@ scrolling_window (struct window *w, int tab_line_p)
  ************************************************************************/
 
 static void
-tty_set_cursor (void)
+tty_set_cursor (struct frame *f)
 {
-  struct frame *f = SELECTED_FRAME ();
-
   if ((cursor_in_echo_area
        /* If we are showing a message instead of the mini-buffer,
 	  show the cursor for the message instead of for the
@@ -5736,7 +5734,7 @@ write_matrix (struct frame *f, bool inhibit_id_p,
 
   /* Now just clean up termcap drivers and set cursor, etc.  */
   if (set_cursor_p)
-    tty_set_cursor ();
+    tty_set_cursor (f);
 }
 
 /* Do line insertions/deletions on frame F for frame-based redisplay.  */
-- 
2.48.1

>From 0e95d664ee498ffd96c372d0d635e1e14fef540c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Wed, 29 Jan 2025 08:34:43 +0100
Subject: [PATCH 2/2] Replace two macros with functions in dispnew.c

* src/dispnew.c (WINDOW_TO_FRAME_VPOS, WINDOW_TO_FRAME_VPOS): Macros
removed.
(window_to_frame_vpos, window_to_frame_vpos): Always define, and use
them instead of the removed macros.
---
 src/dispnew.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index 4b09282121a..c2c41cb5af8 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -140,11 +140,6 @@ check_rows (struct frame *f)
 
 #ifdef GLYPH_DEBUG
 
-static int window_to_frame_vpos (struct window *, int);
-static int window_to_frame_hpos (struct window *, int);
-#define WINDOW_TO_FRAME_VPOS(W, VPOS) window_to_frame_vpos (W, VPOS)
-#define WINDOW_TO_FRAME_HPOS(W, HPOS) window_to_frame_hpos (W, HPOS)
-
 /* One element of the ring buffer containing redisplay history
    information.  */
 
@@ -236,11 +231,6 @@ DEFUN ("dump-redisplay-history", Fdump_redisplay_history,
 }

-#else /* not GLYPH_DEBUG */
-
-#define WINDOW_TO_FRAME_VPOS(W, VPOS) ((VPOS) + WINDOW_TOP_EDGE_LINE (W))
-#define WINDOW_TO_FRAME_HPOS(W, HPOS) ((HPOS) + WINDOW_LEFT_EDGE_COL (W))
-
 #endif /* GLYPH_DEBUG */

@@ -3194,8 +3184,6 @@ check_matrix_pointers (struct glyph_matrix *window_matrix,
 		      VPOS and HPOS translations
  **********************************************************************/
 
-#ifdef GLYPH_DEBUG
-
 /* Translate vertical position VPOS which is relative to window W to a
    vertical position relative to W's frame.  */
 
@@ -3222,9 +3210,6 @@ window_to_frame_hpos (struct window *w, int hpos)
   return hpos;
 }
 
-#endif /* GLYPH_DEBUG */
-
-
 
 /**********************************************************************
 			    Redrawing Frames
@@ -3906,8 +3891,8 @@ abs_cursor_pos (struct frame *f, int *x, int *y)
 	 a new cursor position has been computed.  */
       && w->cursor.vpos < WINDOW_TOTAL_LINES (w))
     {
-      int wx = WINDOW_TO_FRAME_HPOS (w, w->cursor.hpos);
-      int wy = WINDOW_TO_FRAME_VPOS (w, w->cursor.vpos);
+      int wx = window_to_frame_hpos (w, w->cursor.hpos);
+      int wy = window_to_frame_vpos (w, w->cursor.vpos);
 
       wx += max (0, w->left_margin_cols);
 
@@ -5691,8 +5676,8 @@ tty_set_cursor (struct frame *f)
 	     a new cursor position has been computed.  */
 	  && w->cursor.vpos < WINDOW_TOTAL_LINES (w))
 	{
-	  int x = WINDOW_TO_FRAME_HPOS (w, w->cursor.hpos);
-	  int y = WINDOW_TO_FRAME_VPOS (w, w->cursor.vpos);
+	  int x = window_to_frame_hpos (w, w->cursor.hpos);
+	  int y = window_to_frame_vpos (w, w->cursor.vpos);
 
 	  x += max (0, w->left_margin_cols);
 	  cursor_to (f, y, x);
-- 
2.48.1