From d8c821a85196822bc96a4a67df6831bde471758d Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 26 Sep 2023 18:47:01 -0500 Subject: [PATCH 1/4] printoperation: fix case where operation may complete multiple twice If we are the final backend, then after removing ourselves there is no backend remaining. We will schedule the idle even if it has already been scheduled. This idle is required to run exactly once and executing it twices results in a double free that crashes loupe when printing. It also causes the user callback to execute twice, which could cause similar problems. Fixes #6122 --- gtk/print/gtkprintoperation-unix.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/gtk/print/gtkprintoperation-unix.c b/gtk/print/gtkprintoperation-unix.c index db4d878e332..7a6fb04254f 100644 --- a/gtk/print/gtkprintoperation-unix.c +++ b/gtk/print/gtkprintoperation-unix.c @@ -1135,7 +1135,10 @@ printer_list_done_cb (GtkPrintBackend *backend, gtk_print_backend_destroy (backend); g_object_unref (backend); - if (finder->backends == NULL) + /* If there are no more backends left after removing ourselves from the list + * above, then we're finished. + */ + if (finder->backends == NULL && !finder->found_printer) g_idle_add (find_printer_idle, finder); } -- GitLab From dfbafdf047d95e1475806f38e3e8c2fb2db59c1d Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 26 Sep 2023 18:52:37 -0500 Subject: [PATCH 2/4] printoperation: fix another case where operation may complete twice This is a little tricky. At first, I thought we had a codepath where we fail to schedule the idle that completes the print operation: if we take the gtk_print_backend_printer_list_is_done path for each printer backend, then printer_list_done_cb() is never executed and we never schedule the idle. But in fact, in this case, then backends == NULL at the bottom of find_printer(), and we'll schedule the idle there, so it's OK. Except it's not really OK, because we'll schedule it even if a printer was already found, resulting in the callback completing twice and a double free. Simplify this. Schedule the idle in find_printer() only if there are *initially* no backends, not also if all backends are immediately ready and already removed from consideration. Instead, always call printer_list_done_cb() for every backend in find_printer_init(). After the previous commit, printer_list_done_cb() will schedule the idle when appropriate. printer_list_done_cb() additionally disconnects signals that we did not connect in this codepath, but it does so using g_signal_handlers_disconnect_by_func, which is harmless. Otherwise, the only extra work it's doing is scheduling the idle, and that's exactly what find_printer_init() is missing. --- gtk/print/gtkprintoperation-unix.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/gtk/print/gtkprintoperation-unix.c b/gtk/print/gtkprintoperation-unix.c index 7a6fb04254f..4d592eeb6cd 100644 --- a/gtk/print/gtkprintoperation-unix.c +++ b/gtk/print/gtkprintoperation-unix.c @@ -1165,9 +1165,7 @@ find_printer_init (PrinterFinder *finder, if (gtk_print_backend_printer_list_is_done (backend)) { - finder->backends = g_list_remove (finder->backends, backend); - gtk_print_backend_destroy (backend); - g_object_unref (backend); + printer_list_done_cb (backend, finder); } else { @@ -1229,14 +1227,17 @@ find_printer (const char *printer, if (g_module_supported ()) finder->backends = gtk_print_backend_load_modules (); + if (finder->backends == NULL) + { + g_idle_add (find_printer_idle, finder); + return; + } + for (node = finder->backends; !finder->found_printer && node != NULL; node = next) { next = node->next; find_printer_init (finder, GTK_PRINT_BACKEND (node->data)); } - - if (finder->backends == NULL) - g_idle_add (find_printer_idle, finder); } -- GitLab From 653c10e8b6e90604ede1e586a8aa88e4ff46b66e Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 26 Sep 2023 19:25:41 -0500 Subject: [PATCH 3/4] printoperation: add some assertions Let's assert that we schedule the idle callback exactly once. These assertions are not perfect because if the callback executes before we schedule it, then the assertion itself would be a use-after-free, since I'm using the PrinterFinder to track whether the callback that frees it has been scheduled. But in practice when using loupe's print dialog, I was noticing the callback scheduled twice before it was executed. The assertion would have caught this problem. --- gtk/print/gtkprintoperation-unix.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/gtk/print/gtkprintoperation-unix.c b/gtk/print/gtkprintoperation-unix.c index 4d592eeb6cd..ab69e1970be 100644 --- a/gtk/print/gtkprintoperation-unix.c +++ b/gtk/print/gtkprintoperation-unix.c @@ -1056,6 +1056,7 @@ gtk_print_run_page_setup_dialog_async (GtkWindow *parent, struct _PrinterFinder { gboolean found_printer; + gboolean scheduled_callback; GFunc func; gpointer data; char *printer_name; @@ -1088,6 +1089,14 @@ find_printer_idle (gpointer data) return G_SOURCE_REMOVE; } +static void +schedule_finder_callback (PrinterFinder *finder) +{ + g_assert (!finder->scheduled_callback); + g_idle_add (find_printer_idle, finder); + finder->scheduled_callback = TRUE; +} + static void printer_added_cb (GtkPrintBackend *backend, GtkPrinter *printer, @@ -1120,7 +1129,7 @@ printer_added_cb (GtkPrintBackend *backend, } if (finder->found_printer) - g_idle_add (find_printer_idle, finder); + schedule_finder_callback (finder); } static void @@ -1139,7 +1148,7 @@ printer_list_done_cb (GtkPrintBackend *backend, * above, then we're finished. */ if (finder->backends == NULL && !finder->found_printer) - g_idle_add (find_printer_idle, finder); + schedule_finder_callback (finder); } static void @@ -1229,7 +1238,7 @@ find_printer (const char *printer, if (finder->backends == NULL) { - g_idle_add (find_printer_idle, finder); + schedule_finder_callback (finder); return; } -- GitLab From 1e2975147d307728d3ae93e44ae8d934e2dd16a9 Mon Sep 17 00:00:00 2001 From: Michael Catanzaro Date: Tue, 26 Sep 2023 19:28:12 -0500 Subject: [PATCH 4/4] printoperation: fix some strange line spacing --- gtk/print/gtkprintoperation-unix.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtk/print/gtkprintoperation-unix.c b/gtk/print/gtkprintoperation-unix.c index ab69e1970be..cf52c56463b 100644 --- a/gtk/print/gtkprintoperation-unix.c +++ b/gtk/print/gtkprintoperation-unix.c @@ -1249,7 +1249,6 @@ find_printer (const char *printer, } } - GtkPrintOperationResult _gtk_print_operation_platform_backend_run_dialog (GtkPrintOperation *op, gboolean show_dialog, @@ -1261,6 +1260,7 @@ _gtk_print_operation_platform_backend_run_dialog (GtkPrintOperation *op, else return gtk_print_operation_unix_run_dialog (op, show_dialog, parent, do_print); } + void _gtk_print_operation_platform_backend_run_dialog_async (GtkPrintOperation *op, gboolean show_dialog, -- GitLab