From edf5a8783d22466dca32c158683195f207eaf4cf Mon Sep 17 00:00:00 2001 From: John Ralls Date: Wed, 4 Jan 2023 18:16:23 -0800 Subject: [PATCH] Bug 798702 - Crash in gnc_plugin_page_focus_idle_destroy() closing... a report before it completes. The problem was that the progress bar runs the event loop and lets tab get destroyed while the stream handler is running, taking the page and html objects along with it. Then the stream handler returns and the callers try to use the freed page and html objects. Crash. To avoid this take out weak pointers on the page and html and only use the page and html if the respective finalize functions haven't NULLed the weak pointer. --- gnucash/gnome/gnc-plugin-page-report.c | 8 ++- gnucash/html/gnc-html-webkit1.c | 75 ++++++++++++++++---------- gnucash/html/gnc-html-webkit2.c | 39 ++++++++++---- 3 files changed, 84 insertions(+), 38 deletions(-) diff --git a/gnucash/gnome/gnc-plugin-page-report.c b/gnucash/gnome/gnc-plugin-page-report.c index 1ec7b2b194..09bdb946bd 100644 --- a/gnucash/gnome/gnc-plugin-page-report.c +++ b/gnucash/gnome/gnc-plugin-page-report.c @@ -350,6 +350,7 @@ gnc_plugin_page_report_load_uri (GncPluginPage *page) { GncPluginPageReport *report; GncPluginPageReportPrivate *priv; + GncPluginPage *weak_page = page; URLType type; char * id_name; char * child_name; @@ -373,6 +374,7 @@ gnc_plugin_page_report_load_uri (GncPluginPage *page) g_free(id_name); g_free(child_name); + g_object_add_weak_pointer(G_OBJECT(page), (gpointer*)(&weak_page)); gtk_widget_show_all( GTK_WIDGET(priv->container) ); priv->loaded = TRUE; @@ -386,7 +388,11 @@ gnc_plugin_page_report_load_uri (GncPluginPage *page) gnc_html_show_url(priv->html, type, url_location, url_label, 0); g_free(url_location); - gnc_plugin_page_report_set_progressbar( page, FALSE ); + if (weak_page) + { + gnc_plugin_page_report_set_progressbar( page, FALSE ); + g_object_remove_weak_pointer(G_OBJECT(page), (gpointer*)(&weak_page)); + } // this resets the window for the progressbar to NULL gnc_window_set_progressbar_window( NULL ); diff --git a/gnucash/html/gnc-html-webkit1.c b/gnucash/html/gnc-html-webkit1.c index 4d5034caf5..6db5dfc0e5 100644 --- a/gnucash/html/gnc-html-webkit1.c +++ b/gnucash/html/gnc-html-webkit1.c @@ -465,7 +465,7 @@ handle_embedded_object( GncHtmlWebkit* self, gchar* html_str ) * widget. ********************************************************************/ -static void +static gboolean load_to_stream( GncHtmlWebkit* self, URLType type, const gchar* location, const gchar* label ) { @@ -485,30 +485,45 @@ load_to_stream( GncHtmlWebkit* self, URLType type, stream_handler = g_hash_table_lookup( gnc_html_stream_handlers, type ); if ( stream_handler ) { - gboolean ok = stream_handler( location, &fdata, &fdata_len ); + GncHtml *weak_html = GNC_HTML(self); + gboolean ok; - if ( ok ) - { - fdata = fdata ? fdata : g_strdup( "" ); + g_object_add_weak_pointer(G_OBJECT(self), (gpointer *)(&weak_html)); - // Until webkitgtk supports download requests, look for "base.base_location ? priv->base.base_location : "(null)" ); - load_to_stream( GNC_HTML_WEBKIT(self), result.url_type, - new_location, new_label ); + stream_loaded = load_to_stream( GNC_HTML_WEBKIT(self), + result.url_type, + new_location, new_label ); - if ( priv->base.load_cb != NULL ) + if ( stream_loaded && priv->base.load_cb != NULL ) { priv->base.load_cb( GNC_HTML(self), result.url_type, new_location, new_label, priv->base.load_cb_data ); @@ -987,7 +1005,8 @@ impl_webkit_show_url( GncHtml* self, URLType type, /* FIXME : handle new_window = 1 */ gnc_html_history_append( priv->base.history, gnc_html_history_node_new( type, location, label ) ); - load_to_stream( GNC_HTML_WEBKIT(self), type, location, label ); + stream_loaded = load_to_stream( GNC_HTML_WEBKIT(self), type, + location, label ); } while ( FALSE ); @@ -998,7 +1017,7 @@ impl_webkit_show_url( GncHtml* self, URLType type, PERR( "URLType %s not supported.", type ); } - if ( priv->base.load_cb != NULL ) + if ( stream_loaded && priv->base.load_cb != NULL ) { (priv->base.load_cb)( GNC_HTML(self), type, location, label, priv->base.load_cb_data ); } diff --git a/gnucash/html/gnc-html-webkit2.c b/gnucash/html/gnc-html-webkit2.c index a892a4cb90..30925c92a6 100644 --- a/gnucash/html/gnc-html-webkit2.c +++ b/gnucash/html/gnc-html-webkit2.c @@ -466,7 +466,7 @@ handle_embedded_object( GncHtmlWebkit* self, gchar* html_str ) * widget. ********************************************************************/ -static void +static gboolean load_to_stream( GncHtmlWebkit* self, URLType type, const gchar* location, const gchar* label ) { @@ -477,7 +477,7 @@ load_to_stream( GncHtmlWebkit* self, URLType type, DEBUG( "type %s, location %s, label %s", type ? type : "(null)", location ? location : "(null)", label ? label : "(null)"); - g_return_if_fail( self != NULL ); + g_return_val_if_fail( self != NULL, FALSE ); if ( gnc_html_stream_handlers != NULL ) { @@ -486,7 +486,23 @@ load_to_stream( GncHtmlWebkit* self, URLType type, stream_handler = g_hash_table_lookup( gnc_html_stream_handlers, type ); if ( stream_handler ) { - gboolean ok = stream_handler( location, &fdata, &fdata_len ); + GncHtml *weak_html = GNC_HTML(self); + gboolean ok; + + g_object_add_weak_pointer(G_OBJECT(self), + (gpointer*)(&weak_html)); + ok = stream_handler( location, &fdata, &fdata_len ); + + if (!weak_html) // will be NULL if self has been destroyed + { + g_free (fdata); + return FALSE; + } + else + { + g_object_remove_weak_pointer(G_OBJECT(self), + (gpointer*)(&weak_html)); + } if ( ok ) { @@ -533,7 +549,7 @@ load_to_stream( GncHtmlWebkit* self, URLType type, } /* No action required: Webkit jumps to the anchor on its own. */ } - return; + return TRUE; } } @@ -580,7 +596,9 @@ load_to_stream( GncHtmlWebkit* self, URLType type, } } while ( FALSE ); + return TRUE; } + static gboolean perform_navigation_policy (WebKitWebView *web_view, WebKitNavigationPolicyDecision *decision, @@ -782,6 +800,7 @@ impl_webkit_show_url( GncHtml* self, URLType type, GncHTMLUrlCB url_handler; gboolean new_window; GncHtmlWebkitPrivate* priv; + gboolean stream_loaded = FALSE; g_return_if_fail( self != NULL ); g_return_if_fail( GNC_IS_HTML_WEBKIT(self) ); @@ -872,10 +891,11 @@ impl_webkit_show_url( GncHtml* self, URLType type, DEBUG( "resetting base location to %s", priv->base.base_location ? priv->base.base_location : "(null)" ); - load_to_stream( GNC_HTML_WEBKIT(self), result.url_type, - new_location, new_label ); + stream_loaded = load_to_stream( GNC_HTML_WEBKIT(self), + result.url_type, + new_location, new_label ); - if ( priv->base.load_cb != NULL ) + if ( stream_loaded && priv->base.load_cb != NULL ) { priv->base.load_cb( GNC_HTML(self), result.url_type, new_location, new_label, priv->base.load_cb_data ); @@ -938,7 +958,8 @@ impl_webkit_show_url( GncHtml* self, URLType type, /* FIXME : handle new_window = 1 */ gnc_html_history_append( priv->base.history, gnc_html_history_node_new( type, location, label ) ); - load_to_stream( GNC_HTML_WEBKIT(self), type, location, label ); + stream_loaded = load_to_stream( GNC_HTML_WEBKIT(self), + type, location, label ); } while ( FALSE ); @@ -948,7 +969,7 @@ impl_webkit_show_url( GncHtml* self, URLType type, PERR( "URLType %s not supported.", type ); } - if ( priv->base.load_cb != NULL ) + if ( stream_loaded && priv->base.load_cb != NULL ) { (priv->base.load_cb)( GNC_HTML(self), type, location, label, priv->base.load_cb_data ); }