From f3b422f8b7b8db2f2df5b5468e2f4ef6219dd503 Mon Sep 17 00:00:00 2001 From: Philip Van Hoof Date: Mon, 6 Aug 2007 08:17:15 +0000 Subject: [PATCH] 2007-08-06 Philip Van Hoof * GDK lock awareness pmo-trunk-r2935 --- src/dbus_api/modest-dbus-callbacks.c | 54 ++++-- src/maemo/modest-maemo-global-settings-dialog.c | 9 +- src/maemo/modest-maemo-utils.c | 16 +- src/maemo/modest-msg-view-window.c | 14 +- src/maemo/modest-platform.c | 8 +- src/modest-mail-operation.c | 206 +++++++++++------------ src/modest-main.c | 4 +- 7 files changed, 166 insertions(+), 145 deletions(-) diff --git a/src/dbus_api/modest-dbus-callbacks.c b/src/dbus_api/modest-dbus-callbacks.c index 601387d..db002cb 100644 --- a/src/dbus_api/modest-dbus-callbacks.c +++ b/src/dbus_api/modest-dbus-callbacks.c @@ -259,13 +259,17 @@ on_idle_mail_to(gpointer user_data) } else { tny_folder_add_msg (folder, msg, NULL); /* TODO: check err */ - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ ModestWindow *win = modest_msg_edit_window_new (msg, account_name, FALSE); modest_window_mgr_register_window (modest_runtime_get_window_mgr (), win); gtk_widget_show_all (GTK_WIDGET (win)); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ g_object_unref (G_OBJECT(folder)); g_object_unref (win); @@ -364,7 +368,10 @@ on_idle_compose_mail(gpointer user_data) tny_folder_add_msg (folder, msg, NULL); /* TODO: check err */ - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ ModestWindow *win = modest_msg_edit_window_new (msg, account_name, FALSE); @@ -386,7 +393,7 @@ on_idle_compose_mail(gpointer user_data) modest_window_mgr_register_window (modest_runtime_get_window_mgr (), win); gtk_widget_show_all (GTK_WIDGET (win)); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ g_object_unref (G_OBJECT(folder)); g_object_unref (win); @@ -563,8 +570,11 @@ on_idle_open_message (gpointer user_data) msg_uid = modest_tny_folder_get_header_unique_id(header); win_mgr = modest_runtime_get_window_mgr (); - - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ gboolean already_opened = FALSE; ModestWindow *msg_view = NULL; @@ -606,7 +616,7 @@ on_idle_open_message (gpointer user_data) gtk_widget_show_all (GTK_WIDGET (msg_view)); } - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ g_object_unref (header); g_object_unref (account); @@ -722,7 +732,10 @@ on_idle_delete_message (gpointer user_data) error = NULL; res = OSSO_OK; - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ ModestWindow *win = modest_window_mgr_get_main_window (modest_runtime_get_window_mgr ()); modest_do_message_delete (header, win); @@ -740,7 +753,7 @@ on_idle_delete_message (gpointer user_data) modest_ui_actions_refresh_message_window_after_delete (MODEST_MSG_VIEW_WINDOW (msg_view)); } - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ if (header) g_object_unref (header); @@ -802,7 +815,10 @@ on_idle_send_receive(gpointer user_data) { ModestWindow *win; - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ /* Pick the main window if it exists */ win = modest_window_mgr_get_main_window (modest_runtime_get_window_mgr ()); @@ -816,7 +832,7 @@ on_idle_send_receive(gpointer user_data) /* TODO: check the auto-update parameter in the configuration */ modest_ui_actions_do_send_receive_all (win); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ return FALSE; /* Do not call this callback again. */ } @@ -841,7 +857,11 @@ static gboolean on_idle_top_application (gpointer user_data); static gboolean on_idle_open_default_inbox(gpointer user_data) { - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ ModestWindow *win = modest_window_mgr_get_main_window (modest_runtime_get_window_mgr ()); @@ -851,7 +871,7 @@ on_idle_open_default_inbox(gpointer user_data) MODEST_WIDGET_TYPE_FOLDER_VIEW); modest_folder_view_select_first_inbox_or_local (MODEST_FOLDER_VIEW (folder_view)); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ /* This D-Bus method is obviously meant to result in the UI being visible, * so show it, by calling this idle handler directly: */ @@ -876,7 +896,11 @@ static gint on_open_default_inbox(GArray * arguments, gpointer data, osso_rpc_t static gboolean on_idle_top_application (gpointer user_data) { - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ ModestWindow *win = modest_window_mgr_get_main_window (modest_runtime_get_window_mgr ()); @@ -887,7 +911,7 @@ static gboolean on_idle_top_application (gpointer user_data) gtk_window_present (GTK_WINDOW (win)); } - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ return FALSE; /* Do not call this callback again. */ } diff --git a/src/maemo/modest-maemo-global-settings-dialog.c b/src/maemo/modest-maemo-global-settings-dialog.c index 7f3d2ca..9d0d3f5 100644 --- a/src/maemo/modest-maemo-global-settings-dialog.c +++ b/src/maemo/modest-maemo-global-settings-dialog.c @@ -162,9 +162,14 @@ idle_select_default_focus (gpointer data) /* Grab focus, we need to block in order to prevent a recursive call to this callback */ g_signal_handler_block (G_OBJECT (ppriv->notebook), priv->switch_handler); - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ gtk_widget_grab_focus (helper->focus_widget); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ + g_signal_handler_unblock (G_OBJECT (ppriv->notebook), priv->switch_handler); g_free (helper); diff --git a/src/maemo/modest-maemo-utils.c b/src/maemo/modest-maemo-utils.c index d1915fd..61976c9 100644 --- a/src/maemo/modest-maemo-utils.c +++ b/src/maemo/modest-maemo-utils.c @@ -283,9 +283,14 @@ on_idle_secure_auth_finished (gpointer user_data) ModestGetSupportedAuthInfo *info = (ModestGetSupportedAuthInfo*)user_data; /* Operation has finished, close the dialog. Control continues after * gtk_dialog_run in modest_maemo_utils_get_supported_secure_authentication_methods() */ - gdk_threads_enter(); + + /* This is a GDK lock because we are an idle callback and + * the code below is or does Gtk+ code */ + + gdk_threads_enter(); /* CHECKED */ gtk_dialog_response (GTK_DIALOG (info->dialog), GTK_RESPONSE_ACCEPT); - gdk_threads_leave(); + gdk_threads_leave(); /* CHECKED */ + return FALSE; } @@ -299,8 +304,6 @@ on_camel_account_get_supported_secure_authentication ( ModestGetSupportedAuthInfo *info = (ModestGetSupportedAuthInfo*)user_data; g_return_if_fail (info); - gdk_threads_enter(); - /* Free everything if the actual action was canceled */ if (info->cancel) { @@ -347,8 +350,7 @@ on_camel_account_get_supported_secure_authentication ( } tny_iterator_next(iter); } - - g_object_unref(auth_types); + g_object_unref (iter); modest_pair_list_free (pairs); @@ -360,8 +362,6 @@ on_camel_account_get_supported_secure_authentication ( /* Close the dialog in a main thread */ g_idle_add(on_idle_secure_auth_finished, info); } - - gdk_threads_leave(); } static void on_secure_auth_cancel(GtkWidget* dialog, int response, gpointer user_data) diff --git a/src/maemo/modest-msg-view-window.c b/src/maemo/modest-msg-view-window.c index ef442c1..b0a3839 100644 --- a/src/maemo/modest-msg-view-window.c +++ b/src/maemo/modest-msg-view-window.c @@ -1888,21 +1888,27 @@ static gboolean idle_save_mime_part_show_result (SaveMimePartInfo *info) { if (info->pairs != NULL) { - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * save_mime_parts_to_file_with_checks can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ save_mime_parts_to_file_with_checks (info); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ } else { gboolean result; result = info->result; - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * hildon_banner_show_information is or does Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ save_mime_part_info_free (info, TRUE); if (result) { hildon_banner_show_information (NULL, NULL, _CS("sfil_ib_saved")); } else { hildon_banner_show_information (NULL, NULL, _("mail_ib_file_operation_failed")); } - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ } return FALSE; diff --git a/src/maemo/modest-platform.c b/src/maemo/modest-platform.c index b1a8227..cc577d1 100644 --- a/src/maemo/modest-platform.c +++ b/src/maemo/modest-platform.c @@ -971,9 +971,13 @@ on_idle_connect_and_wait(gpointer user_data) printf ("DEBUG: %s:\n", __FUNCTION__); TnyDevice *device = modest_runtime_get_device(); if (!tny_device_is_online (device)) { - gdk_threads_enter(); + + /* This is a GDK lock because we are an idle callback and + * tny_maemo_conic_device_connect can contain Gtk+ code */ + + gdk_threads_enter(); /* CHECKED */ tny_maemo_conic_device_connect (TNY_MAEMO_CONIC_DEVICE (device), NULL); - gdk_threads_leave(); + gdk_threads_leave(); /* CHECKED */ } /* Allow the function that requested this idle callback to continue: */ diff --git a/src/modest-mail-operation.c b/src/modest-mail-operation.c index a84ba17..fef299d 100644 --- a/src/modest-mail-operation.c +++ b/src/modest-mail-operation.c @@ -575,13 +575,12 @@ idle_create_msg_cb (gpointer idle_data) { CreateMsgIdleInfo *info = (CreateMsgIdleInfo *) idle_data; - /* GDK LOCK: this one is here because we call this from a thread */ - /* callback could be 'send mail' or 'save to draft'; i dont knonw */ - /* why these tow operations must serialized, i think it could be */ - /* removed */ - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * info->callback can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ info->callback (info->mail_op, info->msg, info->userdata); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ g_object_unref (info->mail_op); if (info->msg) @@ -1048,18 +1047,14 @@ idle_notify_progress (gpointer data) ModestMailOperationState *state; state = modest_mail_operation_clone_state (mail_op); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers. */ - gdk_threads_enter (); -#endif + + /* This is a GDK lock because we are an idle callback and + * the handlers of this signal can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ g_signal_emit (G_OBJECT (mail_op), signals[PROGRESS_CHANGED_SIGNAL], 0, state, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + gdk_threads_leave (); /* CHECKED */ + g_slice_free (ModestMailOperationState, state); return TRUE; @@ -1077,18 +1072,12 @@ idle_notify_progress_once (gpointer data) pair = (ModestPair *) data; - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers */ -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_enter (); -#endif + /* This is a GDK lock because we are an idle callback and + * the handlers of this signal can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ g_signal_emit (G_OBJECT (pair->first), signals[PROGRESS_CHANGED_SIGNAL], 0, pair->second, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + gdk_threads_leave (); /* CHECKED */ /* Free the state and the reference to the mail operation */ g_slice_free (ModestMailOperationState, (ModestMailOperationState*)pair->second); @@ -1137,22 +1126,23 @@ compare_headers_by_date (gconstpointer a, static gboolean set_last_updated_idle (gpointer data) { - /* GDK LOCK: this one is here because we call this from a thread */ - /* and executed with g_idle_add_full during process of method */ - /* update_account_thread, to serialize access to LAST_UPDATED */ - /* property of each account */ - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * modest_account_mgr_set_int can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED - please recheck */ /* It does not matter if the time is not exactly the same than the time when this idle was called, it's just an approximation and it won't be very different */ + modest_account_mgr_set_int (modest_runtime_get_account_mgr (), (gchar *) data, MODEST_ACCOUNT_LAST_UPDATED, time(NULL), TRUE); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED - please recheck */ return FALSE; } @@ -1164,15 +1154,14 @@ idle_update_account_cb (gpointer data) idle_info = (UpdateAccountInfo *) data; - /* GDK LOCK: this one is here because we call this from a thread */ - /* and executed with g_idle_add during update_account_thread, to */ - /* serialize the execution of refresh user callback (mainloop) */ - /* received as parameter in modest_mail_operation_update_account */ - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * idle_info->callback can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ idle_info->callback (idle_info->mail_op, idle_info->new_headers, idle_info->user_data); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ /* Frees */ g_object_unref (idle_info->mail_op); @@ -1686,18 +1675,14 @@ transfer_folder_status_cb (GObject *obj, priv->total = status->of_total; state = modest_mail_operation_clone_state (self); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers. */ - gdk_threads_enter (); -#endif + + /* This is not a GDK lock because we are a Tinymail callback + * which is already GDK locked by Tinymail */ + + /* no gdk_threads_enter (), CHECKED */ g_signal_emit (G_OBJECT (self), signals[PROGRESS_CHANGED_SIGNAL], 0, state, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + /* no gdk_threads_leave (), CHECKED */ + g_slice_free (ModestMailOperationState, state); } @@ -1740,12 +1725,13 @@ transfer_folder_cb (TnyFolder *folder, /* If user defined callback function was defined, call it */ if (helper->user_callback) { - /* GDK LOCK: I think this one can be removed if Tinymail */ - /* wraps the lock itself. */ - /* This function is called as idle in tinymail. */ - gdk_threads_enter (); + + /* This is not a GDK lock because we are a Tinymail callback + * which is already GDK locked by Tinymail */ + + /* no gdk_threads_enter (), CHECKED */ helper->user_callback (priv->source, helper->user_data); - gdk_threads_leave (); + /* no gdk_threads_leave () , CHECKED */ } /* Free */ @@ -2022,11 +2008,12 @@ get_msg_cb (TnyFolder *folder, /* If user defined callback function was defined, call it even if the operation failed*/ if (helper->user_callback) { - /* This callback is called into an iddle by tinymail, - and idles are not in the main lock */ - gdk_threads_enter (); + /* This is not a GDK lock because we are a Tinymail callback + * which is already GDK locked by Tinymail */ + + /* no gdk_threads_enter (), CHECKED */ helper->user_callback (self, helper->header, msg, helper->user_data); - gdk_threads_leave (); + /* no gdk_threads_leave (), CHECKED */ } /* Notify about operation end */ @@ -2063,18 +2050,14 @@ get_msg_status_cb (GObject *obj, state = modest_mail_operation_clone_state (self); state->bytes_done = status->position; state->bytes_total = status->of_total; -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers. */ - gdk_threads_enter (); -#endif + + /* This is not a GDK lock because we are a Tinymail callback + * which is already GDK locked by Tinymail */ + + /* no gdk_threads_enter (), CHECKED */ g_signal_emit (G_OBJECT (self), signals[PROGRESS_CHANGED_SIGNAL], 0, state, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + /* no gdk_threads_leave (), CHECKED */ + g_slice_free (ModestMailOperationState, state); } @@ -2107,11 +2090,12 @@ notify_get_msgs_full (gpointer data) info = (NotifyGetMsgsInfo *) data; - /* Call the user callback. Idles are not in the main lock, so - lock it */ - gdk_threads_enter (); + /* This is a GDK lock because we are an idle callback and + * because info->user_callback can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ info->user_callback (info->mail_op, info->header, info->msg, info->user_data); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ g_slice_free (NotifyGetMsgsInfo, info); @@ -2130,12 +2114,13 @@ get_msgs_full_destroyer (gpointer data) info = (GetFullMsgsInfo *) data; if (info->notify) { - /* GDK LOCK: this one is here because we call this from */ - /* a thread. */ - /* It's used to free user_data, like pasted attachments */ - gdk_threads_enter (); + + /* This is a GDK lock because we are an idle callback and + * because info->notify can contain Gtk+ code */ + + gdk_threads_enter (); /* CHECKED */ info->notify (info->user_data); - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ } /* free */ @@ -2398,18 +2383,16 @@ transfer_msgs_status_cb (GObject *obj, priv->total = status->of_total; state = modest_mail_operation_clone_state (self); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers. */ - gdk_threads_enter (); -#endif + + /* This is not a GDK lock because we are a Tinymail callback and + * Tinymail already acquires the Gdk lock */ + + /* no gdk_threads_enter (), CHECKED */ + g_signal_emit (G_OBJECT (self), signals[PROGRESS_CHANGED_SIGNAL], 0, state, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + + /* no gdk_threads_leave (), CHECKED */ + g_slice_free (ModestMailOperationState, state); } @@ -2465,12 +2448,12 @@ transfer_msgs_cb (TnyFolder *folder, gboolean cancelled, GError **err, gpointer /* If user defined callback function was defined, call it */ if (helper->user_callback) { - /* GDK LOCK: I think this one can be removed if Tinymail */ - /* wraps the lock itself. */ - /* This function is called as idle in tinymail. */ - gdk_threads_enter (); + /* This is not a GDK lock because we are a Tinymail callback and + * Tinymail already acquires the Gdk lock */ + + /* no gdk_threads_enter (), CHECKED */ helper->user_callback (priv->source, helper->user_data); - gdk_threads_leave (); + /* no gdk_threads_leave (), CHECKED */ } /* Free */ @@ -2615,12 +2598,13 @@ on_refresh_folder (TnyFolder *folder, out: /* Call user defined callback, if it exists */ if (helper->user_callback) { - /* GDK LOCK: I think this one can be removed if Tinymail */ - /* wraps the lock itself. */ - /* This function is called as idle in tinymail. */ - gdk_threads_enter (); + + /* This is not a GDK lock because we are a Tinymail callback and + * Tinymail already acquires the Gdk lock */ + + /* no gdk_threads_enter (), CHECKED */ helper->user_callback (self, folder, helper->user_data); - gdk_threads_leave (); + /* no gdk_threads_leave (), CHECKED */ } /* Free */ @@ -2655,18 +2639,16 @@ on_refresh_folder_status_update (GObject *obj, priv->total = status->of_total; state = modest_mail_operation_clone_state (self); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - /* GDK LOCK: this one is here because we call this from a thread */ - /* However, refresh_account operations call async operations on */ - /* on tinymail, which also emmit 'progress-changed' signal. This */ - /* may be is required to serialize executions of progress-changed'*/ - /* signal handlers. */ - gdk_threads_enter (); -#endif + + /* This is not a GDK lock because we are a Tinymail callback and + * Tinymail already acquires the Gdk lock */ + + /* no gdk_threads_enter (), CHECKED */ + g_signal_emit (G_OBJECT (self), signals[PROGRESS_CHANGED_SIGNAL], 0, state, NULL); -#ifdef TINYMAIL_IDLES_NOT_LOCKED_YET - gdk_threads_leave (); -#endif + + /* no gdk_threads_leave (), CHECKED */ + g_slice_free (ModestMailOperationState, state); } diff --git a/src/modest-main.c b/src/modest-main.c index 8a9fa57..c943088 100644 --- a/src/modest-main.c +++ b/src/modest-main.c @@ -57,7 +57,7 @@ main (int argc, char *argv[]) g_thread_init (NULL); gdk_threads_init (); - gdk_threads_enter (); + gdk_threads_enter (); /* CHECKED */ if (!gtk_init_check(&argc, &argv)) { g_printerr ("modest: failed to initialize gtk\n"); @@ -96,7 +96,7 @@ main (int argc, char *argv[]) retval = 0; cleanup: - gdk_threads_leave (); + gdk_threads_leave (); /* CHECKED */ if (!modest_init_uninit ()) { g_printerr ("modest: modest_init_uninit failed\n"); -- 1.7.9.5