From bc054c94cb46e4f8f8881c2a1b0268e2f05b307b Mon Sep 17 00:00:00 2001 From: Matthew Barnes Date: Tue, 18 May 2010 08:07:19 -0400 Subject: Bug 618902 - Crash when viewing/closing messages quickly Closing an EMailBrowser window causes it to be disposed immediately, but ongoing async operations still hold an EMailBrowser reference -- in particular, regenerating the internal message list and fetching a mail message. The callback functions for these operations were not equipped to deal with the disposed-but-not-yet-finalized object. --- mail/e-mail-browser.c | 3 ++- mail/e-mail-reader.c | 55 ++++++++++++++++++++++++++++++++------------------- mail/message-list.c | 6 ++---- 3 files changed, 39 insertions(+), 25 deletions(-) (limited to 'mail') diff --git a/mail/e-mail-browser.c b/mail/e-mail-browser.c index e237d51173..ca582ccd35 100644 --- a/mail/e-mail-browser.c +++ b/mail/e-mail-browser.c @@ -441,7 +441,8 @@ mail_browser_dispose (GObject *object) } if (priv->message_list != NULL) { - g_object_unref (priv->message_list); + /* This will cancel a regen operation. */ + gtk_widget_destroy (priv->message_list); priv->message_list = NULL; } diff --git a/mail/e-mail-reader.c b/mail/e-mail-reader.c index 5729486201..1293730a63 100644 --- a/mail/e-mail-reader.c +++ b/mail/e-mail-reader.c @@ -52,7 +52,8 @@ #include "mail/message-list.h" #define E_MAIL_READER_GET_PRIVATE(obj) \ - (mail_reader_get_private (G_OBJECT (obj))) + ((EMailReaderPrivate *) g_object_get_qdata \ + (G_OBJECT (obj), quark_private)) typedef struct _EMailReaderPrivate EMailReaderPrivate; @@ -92,31 +93,24 @@ static GQuark quark_private; static guint signals[LAST_SIGNAL]; static void -mail_reader_finalize (EMailReaderPrivate *priv) +mail_reader_destroy (GObject *object) { - if (priv->message_selected_timeout_id > 0) - g_source_remove (priv->message_selected_timeout_id); - - g_free (priv->mark_read_message_uid); - - g_slice_free (EMailReaderPrivate, priv); + /* This will free the private struct. */ + g_object_set_qdata (object, quark_private, NULL); } -static EMailReaderPrivate * -mail_reader_get_private (GObject *object) +static void +mail_reader_private_free (EMailReaderPrivate *priv) { - EMailReaderPrivate *priv; + if (priv->message_selected_timeout_id > 0) + g_source_remove (priv->message_selected_timeout_id); - priv = g_object_get_qdata (object, quark_private); + if (priv->retrieving_message_operation_id > 0) + mail_msg_cancel (priv->retrieving_message_operation_id); - if (G_UNLIKELY (priv == NULL)) { - priv = g_slice_new0 (EMailReaderPrivate); - g_object_set_qdata_full ( - object, quark_private, priv, - (GDestroyNotify) mail_reader_finalize); - } + g_free (priv->mark_read_message_uid); - return priv; + g_slice_free (EMailReaderPrivate, priv); } static void @@ -1811,6 +1805,14 @@ mail_reader_message_loaded_cb (CamelFolder *folder, priv = E_MAIL_READER_GET_PRIVATE (reader); + /* If the private struct is NULL, the EMailReader was destroyed + * while we were loading the message and we're likely holding the + * last reference. Nothing to do but drop the reference. */ + if (priv == NULL) { + g_object_unref (reader); + return; + } + html_display = e_mail_reader_get_html_display (reader); message_list = e_mail_reader_get_message_list (reader); @@ -2540,7 +2542,7 @@ e_mail_reader_get_type (void) type = g_type_register_static ( G_TYPE_INTERFACE, "EMailReader", &type_info, 0); - g_type_interface_add_prerequisite (type, G_TYPE_OBJECT); + g_type_interface_add_prerequisite (type, GTK_TYPE_OBJECT); } return type; @@ -2689,6 +2691,19 @@ e_mail_reader_init (EMailReader *reader) g_signal_connect_swapped ( message_list, "selection-change", G_CALLBACK (e_mail_reader_changed), reader); + + /* Install a private struct for storing things like flags and + * timeout and asynchronous operation IDs. We delete it when + * the EMailReader is destroyed rather than finalized so that + * asynchronous callbacks holding a reference can detect that + * the reader has been destroyed and drop their reference. */ + g_object_set_qdata_full ( + G_OBJECT (reader), quark_private, + g_slice_new0 (EMailReaderPrivate), + (GDestroyNotify) mail_reader_private_free); + g_signal_connect ( + reader, "destroy", + G_CALLBACK (mail_reader_destroy), NULL); } void diff --git a/mail/message-list.c b/mail/message-list.c index e09ac09b77..466d4c5777 100644 --- a/mail/message-list.c +++ b/mail/message-list.c @@ -4580,7 +4580,7 @@ mail_regen_list (MessageList *ml, const gchar *search, const gchar *hideexpr, Ca #endif m = mail_msg_new (®en_list_info); - m->ml = ml; + m->ml = g_object_ref (ml); m->search = g_strdup (search); m->hideexpr = g_strdup (hideexpr); m->changes = changes; @@ -4588,9 +4588,7 @@ mail_regen_list (MessageList *ml, const gchar *search, const gchar *hideexpr, Ca m->hidedel = ml->hidedeleted; m->hidejunk = ml->hidejunk; m->thread_subject = gconf_client_get_bool (gconf, "/apps/evolution/mail/display/thread_subject", NULL); - g_object_ref(ml); - m->folder = ml->folder; - g_object_ref (m->folder); + m->folder = g_object_ref (ml->folder); m->last_row = -1; m->expand_state = NULL; -- cgit