Subject: Re: bug#76406: 31.0.50; Crash with tty child frames

From: Gerd Möllmann <[email-masked]>
Date: Sun, 23 Feb 2025 05:13:54 +0100

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

> martin rudalics <[email-masked]> writes:
>
>>> So I guess I'm missing something important. Any ideas?
>>
>> Not really: I do the same but positioned the yellow frame more to the
>> left so that it is not obscured by the other two child frames.  Both
>> tty-child-frames.el ("/home/martin/temp/tm.el" is my location of your
>> tm.el) and my patch are attached.  I also attach a screenshot
>> immediately before clicking on the Buffer menu.  A complete transcript
>> of the debugging session is below.
>
> So, I think I have now exactly what you are using, and I eventually
> got a segfault somehow, and found out something:
>
> The root frame ended up containing glyphs from a dead menu frame. The
> dead frame's face cache was set to NULL because it had been freed. The
> series of events leading to this is still unclear to me. At least we
> never copy glyphs from a dead frame, that much I can say.
>
> Doesn't look like the assertion you are seeing, but maybe it's somehow
> related.
>
> More in the next round.

Could you please give this a try, Martin?

>From 4ab7912a5521e11668c19d31d2925bad50cbdd02 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Gerd=20M=C3=B6llmann?= <[email-masked]>
Date: Sun, 23 Feb 2025 05:07:55 +0100
Subject: [PATCH] Enforce redisplay when deleting a child frame (bug#76406)

* src/term.c (tty_free_frame_resources): When deleting a child mark its
root frame to garbaged.
* src/dispnew.c (prepare_desired_root_row): Add a check for GLYPH_DEBUG.
---
 src/dispnew.c | 18 ++++++++++++++----
 src/term.c    |  8 +++++++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/src/dispnew.c b/src/dispnew.c
index c222e721077..cd0ed7e6414 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -3514,7 +3514,7 @@ make_matrix_current (struct frame *f)
 
 /* Prepare ROOT's desired row at index Y for copying child frame
    contents to it.  Value is the prepared desired row or NULL if we
-   don't have, and can't contruct a desired row.  */
+   don't have, and can't construct a desired row.  */
 
 static struct glyph_row *
 prepare_desired_root_row (struct frame *root, int y)
@@ -3525,12 +3525,22 @@ prepare_desired_root_row (struct frame *root, int y)
     return desired_row;
 
   /* If we have a current row that is up to date, copy that to the
-     desired row and use that.  */
-  /* Don't copy rows that aren't enabled, in particuler because they
-     might not have the 'frame' member of glyphs set.  */
+     desired row and use that.  Don't copy rows that are bot enabled, in
+     particular because they might not have the 'frame' member of glyphs
+     set.  */
   struct glyph_row *current_row = MATRIX_ROW (root->current_matrix, y);
   if (current_row->enabled_p)
     {
+# ifdef GLYPH_DEBUG
+      /* Safety belt: Try to make sure that we don't copy glyphs from a
+         stale current matrix that contains glyphs referring to dead
+         frames. */
+      for (int i = 0; i < current_row->used[TEXT_AREA]; ++i)
+	{
+	  struct glyph *glyph = current_row->glyphs[TEXT_AREA] + i;
+	  eassert (glyph->frame && FRAME_LIVE_P (glyph->frame));
+	}
+# endif
       memcpy (desired_row->glyphs[0], current_row->glyphs[0],
 	      root->current_matrix->matrix_w * sizeof (struct glyph));
       desired_row->enabled_p = true;
diff --git a/src/term.c b/src/term.c
index 2ef763d0ffc..5b45aa4907a 100644
--- a/src/term.c
+++ b/src/term.c
@@ -4134,7 +4134,8 @@ create_tty_output (struct frame *f)
   f->output_data.tty = t;
 }
 
-/* Delete frame F's face cache, and its tty-dependent part.  */
+/* Delete frame F's face cache, and its tty-dependent part.  This is
+   installed as a delete_frame_hook.  */
 
 static void
 tty_free_frame_resources (struct frame *f)
@@ -4142,6 +4143,11 @@ tty_free_frame_resources (struct frame *f)
   eassert (FRAME_TERMCAP_P (f));
   free_frame_faces (f);
   xfree (f->output_data.tty);
+
+  /* Deleting a child frame means we have to thoroughly redisplay its
+     root frame to make sure the child disappears from the display.  */
+  if (FRAME_PARENT_FRAME (f))
+    SET_FRAME_GARBAGED (root_frame (f));
 }
 
 #elif defined MSDOS
-- 
2.48.1