Re: MPS: Crash while completing symbol with corfu

[Pip Cet <pipcet@protonmail.com>]

[-- Attachment #1: Type: text/plain, Size: 653 bytes --]

On Tuesday, July 2nd, 2024 at 11:38, Ihor Radchenko <yantar92@posteo.net> wrote:
> Pip Cet pipcet@protonmail.com writes:
> 
> > > I confirm that it was an input with tooltip displayed (that's what corfu does).
> > 
> > Ihor, is this one reproducible? If it is, could you try with this patch installed?
> 
> 
> This crashes on startup:

Oops, turns out you shouldn't double free just to be sure.

Can you try the attached patch? I was able to trigger your bug by running

./emacs -Q --eval "(while t (menu-bar-mode 'toggle) (sit-for 0.1))"

The change is in menu_destroy_callback, which passed through the wrong pointer.

Thanks!

Pip

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-try-to-fix-GTK-related-crashes.patch --]
[-- Type: text/x-patch; name=0002-try-to-fix-GTK-related-crashes.patch, Size: 12584 bytes --]

From 3e34d7b26e2d1d36025094cdbc2d19a3435b9ae1 Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@protonmail.com>
Date: Tue, 2 Jul 2024 00:18:43 +0000
Subject: [PATCH] Try to fix GTK-related crashes.

---
 src/gtkutil.c | 115 +++++++++++++++++++++++++++++++++++---------------
 src/xmenu.c   |   4 +-
 2 files changed, 83 insertions(+), 36 deletions(-)

diff --git a/src/gtkutil.c b/src/gtkutil.c
index e0bae6bf1ae..25dc27d5aec 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -149,6 +149,17 @@ #define TB_INFO_KEY "xg_frame_tb_info"
 
 \f
 
+#define glib_user_data(o) ({				\
+      gpointer p = igc_xzalloc_ambig (sizeof (o));	\
+      memcpy (p, &(o), sizeof (o));			\
+      p;						\
+    })
+
+static void glib_free_user_data (gpointer data, GClosure *closure)
+{
+  igc_xfree (data);
+}
+
 #ifdef HAVE_GTK3
 static void
 emacs_menu_bar_init (EmacsMenuBar *menu_bar)
@@ -828,7 +839,7 @@ hierarchy_ch_cb (GtkWidget *widget,
                  GtkWidget *previous_toplevel,
                  gpointer   user_data)
 {
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
   xp_output *x = f->output_data.xp;
   GtkWidget *top = gtk_widget_get_toplevel (x->ttip_lbl);
 
@@ -850,7 +861,7 @@ qttip_cb (GtkWidget  *widget,
           GtkTooltip *tooltip,
           gpointer    user_data)
 {
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
   xp_output *x = f->output_data.xp;
   if (x->ttip_widget == NULL)
     {
@@ -882,8 +893,11 @@ qttip_cb (GtkWidget  *widget,
       gtk_widget_realize (GTK_WIDGET (x->ttip_window));
       gtk_widget_realize (x->ttip_lbl);
 
-      g_signal_connect (x->ttip_lbl, "hierarchy-changed",
-                        G_CALLBACK (hierarchy_ch_cb), f);
+      g_signal_connect_data (x->ttip_lbl, "hierarchy-changed",
+			     G_CALLBACK (hierarchy_ch_cb),
+			     glib_user_data (f),
+			     glib_free_user_data,
+			     G_CONNECT_DEFAULT);
     }
 
   return FALSE;
@@ -1556,8 +1570,11 @@ xg_create_frame_widgets (struct frame *f)
 
   gtk_widget_set_app_paintable (wtop, f->alpha_background != 1.0);
 #if GTK_CHECK_VERSION (3, 10, 0)
-  g_signal_connect (G_OBJECT (wtop), "style-updated",
-		    G_CALLBACK (xg_widget_style_updated), f);
+  g_signal_connect_data (G_OBJECT (wtop), "style-updated",
+			 G_CALLBACK (xg_widget_style_updated),
+			 glib_user_data (f),
+			 glib_free_user_data,
+			 G_CONNECT_DEFAULT);
 #endif
 
   /* gtk_window_set_has_resize_grip is a Gtk+ 3.0 function but Ubuntu
@@ -1646,8 +1663,11 @@ xg_create_frame_widgets (struct frame *f)
 #ifndef HAVE_PGTK
   /* Add callback to do nothing on WM_DELETE_WINDOW.  The default in
      GTK is to destroy the widget.  We want Emacs to do that instead.  */
-  g_signal_connect (G_OBJECT (wtop), "delete-event",
-                    G_CALLBACK (delete_cb), f);
+  g_signal_connect_data (G_OBJECT (wtop), "delete-event",
+			 G_CALLBACK (delete_cb),
+			 glib_user_data (f),
+			 glib_free_user_data,
+			 G_CONNECT_DEFAULT);
 #endif
 
   /* Convert our geometry parameters into a geometry string
@@ -1735,14 +1755,18 @@ xg_create_frame_widgets (struct frame *f)
   f->output_data.xp->ttip_window = 0;
 #ifndef HAVE_PGTK
   gtk_widget_set_tooltip_text (wtop, "Dummy text");
-  g_signal_connect (wtop, "query-tooltip", G_CALLBACK (qttip_cb), f);
+  g_signal_connect_data (wtop, "query-tooltip", G_CALLBACK (qttip_cb),
+			 glib_user_data (f), glib_free_user_data,
+			 G_CONNECT_DEFAULT);
 
   imc = gtk_im_multicontext_new ();
   g_object_ref (imc);
   gtk_im_context_set_use_preedit (imc, TRUE);
 
-  g_signal_connect (G_OBJECT (imc), "commit",
-		    G_CALLBACK (xg_im_context_commit), f);
+  g_signal_connect_data (G_OBJECT (imc), "commit",
+		    G_CALLBACK (xg_im_context_commit),
+		    glib_user_data (f), glib_free_user_data,
+		    G_CONNECT_DEFAULT);
   g_signal_connect (G_OBJECT (imc), "preedit-changed",
 		    G_CALLBACK (xg_im_context_preedit_changed), NULL);
   g_signal_connect (G_OBJECT (imc), "preedit-end",
@@ -1853,7 +1877,9 @@ xg_create_frame_outer_widgets (struct frame *f)
   f->output_data.xp->ttip_window = 0;
 #ifndef HAVE_PGTK
   gtk_widget_set_tooltip_text (wtop, "Dummy text");
-  g_signal_connect (wtop, "query-tooltip", G_CALLBACK (qttip_cb), f);
+  g_signal_connect_data (wtop, "query-tooltip", G_CALLBACK (qttip_cb),
+		    glib_user_data (f), glib_free_user_data,
+		    G_CONNECT_DEFAULT);
 #endif
 
   {
@@ -2371,8 +2397,11 @@ create_dialog (widget_value *wv,
           if (! item->enabled)
             gtk_widget_set_sensitive (w, FALSE);
           if (select_cb)
-            g_signal_connect (G_OBJECT (w), "clicked",
-                              select_cb, item->call_data);
+            g_signal_connect_data (G_OBJECT (w), "clicked",
+                              select_cb,
+			      glib_user_data (item->call_data),
+			      glib_free_user_data,
+			      G_CONNECT_DEFAULT);
 
           gtk_box_pack_start (cur_box, w, TRUE, TRUE, button_spacing);
           if (++button_nr == left_buttons)
@@ -3082,9 +3111,9 @@ xg_mark_data (void)
 static void
 menuitem_destroy_callback (GtkWidget *w, gpointer client_data)
 {
-  if (client_data)
+  xg_menu_item_cb_data *data = *(xg_menu_item_cb_data **)client_data;
+  if (data)
     {
-      xg_menu_item_cb_data *data = client_data;
       xg_list_remove (&xg_menu_item_cb_list, &data->ptrs);
       xfree (data);
     }
@@ -3129,7 +3158,7 @@ menuitem_highlight_callback (GtkWidget *w,
 static void
 menu_destroy_callback (GtkWidget *w, gpointer client_data)
 {
-  unref_cl_data (client_data);
+  unref_cl_data (*(xg_menu_cb_data **)client_data);
 }
 
 /* Make a GTK widget that contains both UTF8_LABEL and UTF8_KEY (both
@@ -3277,10 +3306,12 @@ xg_create_one_menuitem (widget_value *item,
   cb_data->cl_data = cl_data;
   cb_data->call_data = item->call_data;
 
-  g_signal_connect (G_OBJECT (w),
+  g_signal_connect_data (G_OBJECT (w),
                     "destroy",
                     G_CALLBACK (menuitem_destroy_callback),
-                    cb_data);
+		    glib_user_data (cb_data),
+		    glib_free_user_data,
+		    G_CONNECT_DEFAULT);
 
   /* Put cb_data in widget, so we can get at it when modifying menubar  */
   g_object_set_data (G_OBJECT (w), XG_ITEM_DATA, cb_data);
@@ -3290,7 +3321,10 @@ xg_create_one_menuitem (widget_value *item,
     {
       if (select_cb)
         cb_data->select_id
-          = g_signal_connect (G_OBJECT (w), "activate", select_cb, cb_data);
+          = g_signal_connect_data (G_OBJECT (w), "activate", select_cb,
+			      glib_user_data (cb_data),
+			      glib_free_user_data,
+			      G_CONNECT_DEFAULT);
     }
 
   return w;
@@ -3378,8 +3412,10 @@ create_menus (widget_value *data,
 #endif
 
 #ifdef HAVE_PGTK
-	  g_signal_connect (G_OBJECT (wmenu), "button-press-event",
-			    G_CALLBACK (menu_bar_button_pressed_cb), f);
+	  g_signal_connect_data (G_OBJECT (wmenu), "button-press-event",
+			    G_CALLBACK (menu_bar_button_pressed_cb),
+			    glib_user_data (f), glib_free_user_data,
+			    G_CONNECT_DEFAULT);
 #endif
           /* Set width of menu bar to a small value so it doesn't enlarge
              a small initial frame size.  The width will be set to the
@@ -3391,8 +3427,10 @@ create_menus (widget_value *data,
       /* Put cl_data on the top menu for easier access.  */
       cl_data = make_cl_data (cl_data, f, highlight_cb);
       g_object_set_data (G_OBJECT (wmenu), XG_FRAME_DATA, (gpointer)cl_data);
-      g_signal_connect (G_OBJECT (wmenu), "destroy",
-                        G_CALLBACK (menu_destroy_callback), cl_data);
+      g_signal_connect_data (G_OBJECT (wmenu), "destroy",
+                        G_CALLBACK (menu_destroy_callback),
+			glib_user_data (cl_data), glib_free_user_data,
+			G_CONNECT_DEFAULT);
 
       if (name)
         gtk_widget_set_name (wmenu, name);
@@ -3864,8 +3902,11 @@ xg_update_menu_item (widget_value *val,
           /* This item shall have a select callback.  */
           if (! cb_data->select_id)
             cb_data->select_id
-              = g_signal_connect (G_OBJECT (w), "activate",
-                                  select_cb, cb_data);
+              = g_signal_connect_data (G_OBJECT (w), "activate",
+                                  select_cb,
+				  glib_user_data (cb_data),
+				  glib_free_user_data,
+				  G_CONNECT_DEFAULT);
         }
       else if (cb_data->select_id)
         {
@@ -4111,7 +4152,7 @@ xg_modify_menubar_widgets (GtkWidget *menubar, struct frame *f,
 menubar_map_cb (GtkWidget *w, gpointer user_data)
 {
   GtkRequisition req;
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
   gtk_widget_get_preferred_size (w, NULL, &req);
   req.height *= xg_get_scale (f);
   if (FRAME_MENUBAR_HEIGHT (f) != req.height)
@@ -4143,7 +4184,9 @@ xg_update_frame_menubar (struct frame *f)
                       FALSE, FALSE, 0);
   gtk_box_reorder_child (GTK_BOX (x->vbox_widget), x->menubar_widget, 0);
 
-  g_signal_connect (x->menubar_widget, "map", G_CALLBACK (menubar_map_cb), f);
+  g_signal_connect_data (x->menubar_widget, "map", G_CALLBACK (menubar_map_cb),
+		    glib_user_data (f), glib_free_user_data,
+		    G_CONNECT_DEFAULT);
   gtk_widget_show_all (x->menubar_widget);
   gtk_widget_get_preferred_size (x->menubar_widget, NULL, &req);
   req.height *= scale;
@@ -5246,7 +5289,9 @@ xg_print_frames_dialog (Lisp_Object frames)
   if (page_setup != NULL)
     gtk_print_operation_set_default_page_setup (print, page_setup);
   gtk_print_operation_set_n_pages (print, list_length (frames));
-  g_signal_connect (print, "draw-page", G_CALLBACK (draw_page), &frames);
+  g_signal_connect_data (print, "draw-page", G_CALLBACK (draw_page),
+			 glib_user_data (frames), glib_free_user_data,
+			 G_CONNECT_DEFAULT);
   res = gtk_print_operation_run (print, GTK_PRINT_OPERATION_ACTION_PRINT_DIALOG,
                                  NULL, NULL);
   if (res == GTK_PRINT_OPERATION_RESULT_APPLY)
@@ -5478,7 +5523,7 @@ tb_size_cb (GtkWidget    *widget,
   /* When tool bar is created it has one preferred size.  But when size is
      allocated between widgets, it may get another.  So we must update
      size hints if tool bar size changes.  Seen on Fedora 18 at least.  */
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
 
   if (xg_update_tool_bar_sizes (f))
     adjust_frame_size (f, -1, -1, 2, false, Qtool_bar_lines);
@@ -5521,8 +5566,10 @@ xg_create_tool_bar (struct frame *f)
   gtk_toolbar_set_style (GTK_TOOLBAR (x->toolbar_widget), GTK_TOOLBAR_ICONS);
   gtk_orientable_set_orientation (GTK_ORIENTABLE (x->toolbar_widget),
                                   GTK_ORIENTATION_HORIZONTAL);
-  g_signal_connect (x->toolbar_widget, "size-allocate",
-                    G_CALLBACK (tb_size_cb), f);
+  g_signal_connect_data (x->toolbar_widget, "size-allocate",
+			 G_CALLBACK (tb_size_cb),
+			 glib_user_data (f), glib_free_user_data,
+			 G_CONNECT_DEFAULT);
 #ifdef HAVE_GTK3
   gsty = gtk_widget_get_style_context (x->toolbar_widget);
   gtk_style_context_add_class (gsty, "primary-toolbar");
@@ -6337,7 +6384,7 @@ xg_virtual_mods_to_x (struct x_display_info *dpyinfo, guint virtual)
 xg_im_context_commit (GtkIMContext *imc, gchar *str,
 		      gpointer user_data)
 {
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
   struct input_event ie;
 #ifdef HAVE_XINPUT2
   struct xi_device_t *source;
@@ -6708,7 +6755,7 @@ xg_filter_key (struct frame *frame, XEvent *xkey)
 static void
 xg_widget_style_updated (GtkWidget *widget, gpointer user_data)
 {
-  struct frame *f = user_data;
+  struct frame *f = *(struct frame **)user_data;
 
   if (f->alpha_background < 1.0)
     {
diff --git a/src/xmenu.c b/src/xmenu.c
index 6dd7b3f37a0..f318db6f433 100644
--- a/src/xmenu.c
+++ b/src/xmenu.c
@@ -793,7 +793,7 @@ menu_highlight_callback (Widget widget, LWLIB_ID id, void *call_data)
 static void
 menubar_selection_callback (GtkWidget *widget, gpointer client_data)
 {
-  xg_menu_item_cb_data *cb_data = client_data;
+  xg_menu_item_cb_data *cb_data = *(xg_menu_item_cb_data **)client_data;
 
   if (xg_crazy_callback_abort)
     return;
@@ -1478,7 +1478,7 @@ menu_position_func (GtkMenu *menu, gint *x, gint *y, gboolean *push_in, gpointer
 static void
 popup_selection_callback (GtkWidget *widget, gpointer client_data)
 {
-  xg_menu_item_cb_data *cb_data = client_data;
+  xg_menu_item_cb_data *cb_data = *(xg_menu_item_cb_data **)client_data;
 
   if (xg_crazy_callback_abort) return;
   if (cb_data) menu_item_selection = cb_data->call_data;
-- 
2.45.2