Re: [PATCH v7 1/3] tools: port perf ui from GTK 2 to GTK 4

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Connecting to the removed 'destroy' signal in GTK4 causes main loop hangs and use-after-free/double-free bugs.
--

commit fc78f437c6f22aa072d608cee2f291c6178f6853
Author: Matt Turner <mattst88@gmail.com>

    tools: port perf ui from GTK 2 to GTK 4
    
    Port straight to GTK 4 rather than GTK 3, since GTK 4 is where new
    development happens and GTK 3 is old itself now.

> diff --git a/tools/perf/ui/gtk/progress.c b/tools/perf/ui/gtk/progress.c
> index eea6fcde518a8..770f9251b54b7 100644
> --- a/tools/perf/ui/gtk/progress.c
> +++ b/tools/perf/ui/gtk/progress.c
> @@ -1,49 +1,65 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <inttypes.h>
> +#include <stdio.h>

[ ... ]

> +static void gtk_ui_progress__destroyed(GtkWidget *widget __maybe_unused,
> +					gpointer data __maybe_unused)
> +{
> +	dialog = NULL;
> +	progress = NULL;
> +}
> +
>  static void gtk_ui_progress__update(struct ui_progress *p)
>  {
>  	double fraction = p->total ? 1.0 * p->curr / p->total : 0.0;
>  	char buf[1024];
>  
>  	if (dialog == NULL) {
> -		GtkWidget *vbox = gtk_vbox_new(TRUE, 5);
> +		GtkWidget *vbox = gtk_box_new(GTK_ORIENTATION_VERTICAL, 5);
>  		GtkWidget *label = gtk_label_new(p->title);
>  
> -		dialog = gtk_window_new(GTK_WINDOW_TOPLEVEL);
> +		dialog = gtk_window_new();
>  		progress = gtk_progress_bar_new();
>  
> -		gtk_box_pack_start(GTK_BOX(vbox), label, TRUE, FALSE, 3);
> -		gtk_box_pack_start(GTK_BOX(vbox), progress, TRUE, TRUE, 3);
> +		gtk_widget_set_vexpand(label, TRUE);
> +		gtk_box_append(GTK_BOX(vbox), label);
> +		gtk_widget_set_vexpand(progress, TRUE);
> +		gtk_box_append(GTK_BOX(vbox), progress);
> +
> +		gtk_window_set_child(GTK_WINDOW(dialog), vbox);
>  
> -		gtk_container_add(GTK_CONTAINER(dialog), vbox);
> +		g_signal_connect(dialog, "destroy",
> +				 G_CALLBACK(gtk_ui_progress__destroyed), NULL);

[Severity: Medium]
In gtk_ui_progress__update(), will this signal connection fail at runtime?
The destroy signal was removed from GtkWidget in GTK 4. If it fails silently,
gtk_ui_progress__destroyed() would not be called to nullify the pointer when
the user manually closes the progress window. This could result in a
use-after-free or double-free crash when gtk_ui_progress__finish() is
subsequently called on the already-freed pointer.

[ ... ]

> diff --git a/tools/perf/ui/gtk/util.c b/tools/perf/ui/gtk/util.c
> index c47f5c387838c..5823ff0d7c2a7 100644
> --- a/tools/perf/ui/gtk/util.c
> +++ b/tools/perf/ui/gtk/util.c
> @@ -28,86 +30,103 @@ int perf_gtk__deactivate_context(struct perf_gtk_context **ctx)

[ ... ]

> +	/*
> +	 * "response" only fires when a button is clicked; DESTROY_WITH_PARENT
> +	 * destroys the dialog directly without it. Quit from "destroy"
> +	 * instead, which fires either way, so the nested loop below can't
> +	 * outlive the dialog and hang.
> +	 */
> +	loop = g_main_loop_new(NULL, FALSE);
> +	perf_gtk__error_loops = g_slist_prepend(perf_gtk__error_loops, loop);
> +	g_signal_connect(dialog, "response",
> +			 G_CALLBACK(perf_gtk__dialog_response), NULL);
> +	g_signal_connect_swapped(dialog, "destroy",
> +				 G_CALLBACK(g_main_loop_quit), loop);

[Severity: Medium]
In perf_gtk__error(), since the destroy signal no longer exists in GTK 4,
could this leave the nested main loop running indefinitely and hanging the
application? Because the destroy signal is not emitted when the dialog is
closed (even via the confirmation button), g_main_loop_quit() will never be
called.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-perf-gtk2-v7-0-1ece839fbca0@gmail.com?part=1