gnucash maint: Bug 798702 - Crash in gnc_plugin_page_focus_idle_destroy() closing...
John Ralls
jralls at code.gnucash.org
Wed Jan 4 21:21:53 EST 2023
Updated via https://github.com/Gnucash/gnucash/commit/edf5a878 (commit)
from https://github.com/Gnucash/gnucash/commit/1085d892 (commit)
commit edf5a8783d22466dca32c158683195f207eaf4cf
Author: John Ralls <jralls at ceridwen.us>
Date: Wed Jan 4 18:16:23 2023 -0800
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.
diff --git a/gnucash/gnome/gnc-plugin-page-report.c b/gnucash/gnome/gnc-plugin-page-report.c
index 1ec7b2b19..09bdb946b 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 4d5034caf..6db5dfc0e 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 );
-
- if ( ok )
- {
- fdata = fdata ? fdata : g_strdup( "" );
-
- // Until webkitgtk supports download requests, look for "<object classid="
- // indicating the beginning of an embedded graph. If found, handle it
- if ( g_strstr_len( fdata, -1, "<object classid=" ) != NULL )
- {
- gchar* new_fdata;
- new_fdata = handle_embedded_object( self, fdata );
- g_free( fdata );
- fdata = new_fdata;
- }
+ 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) {
+ fdata = fdata ? fdata : g_strdup("");
+
+ // Until webkitgtk supports download requests, look for "<object
+ // classid=" indicating the beginning of an embedded graph. If
+ // found, handle it
+ if (g_strstr_len(fdata, -1, "<object classid=") != NULL) {
+ gchar *new_fdata;
+ new_fdata = handle_embedded_object(self, fdata);
+ g_free(fdata);
+ fdata = new_fdata;
+ }
- // Save a copy for export purposes
- if ( priv->html_string != NULL )
- {
- g_free( priv->html_string );
- }
- priv->html_string = g_strdup( fdata );
- impl_webkit_show_data( GNC_HTML(self), fdata, strlen(fdata) );
-// webkit_web_view_load_html_string( priv->web_view, fdata, BASE_URI_NAME );
+ // Save a copy for export purposes
+ if (priv->html_string != NULL) {
+ g_free(priv->html_string);
+ }
+ priv->html_string = g_strdup(fdata);
+ impl_webkit_show_data(GNC_HTML(self), fdata, strlen(fdata));
+ // webkit_web_view_load_html_string( priv->web_view,
+ // fdata, BASE_URI_NAME );
}
else
{
@@ -529,7 +544,7 @@ load_to_stream( GncHtmlWebkit* self, URLType type,
/* No action required: Webkit jumps to the anchor on its own. */
}
- return;
+ return TRUE;
}
}
@@ -578,6 +593,7 @@ load_to_stream( GncHtmlWebkit* self, URLType type,
}
while ( FALSE );
+ return TRUE;
}
#if 0
@@ -831,6 +847,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) );
@@ -921,10 +938,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 );
@@ -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 a892a4cb9..30925c92a 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 );
}
Summary of changes:
gnucash/gnome/gnc-plugin-page-report.c | 8 +++-
gnucash/html/gnc-html-webkit1.c | 79 +++++++++++++++++++++-------------
gnucash/html/gnc-html-webkit2.c | 39 +++++++++++++----
3 files changed, 86 insertions(+), 40 deletions(-)
More information about the gnucash-changes
mailing list