Patch: prevent send queue to launch two sending threads
- From: Sergio Villar Senin <svillar igalia com>
- To: tinymail-devel-list <tinymail-devel-list gnome org>
- Subject: Patch: prevent send queue to launch two sending threads
- Date: Wed, 19 Nov 2008 16:45:32 +0100
Hi,
this is a fix that we developed for the stable branch of tinymail and
that is worth merging to trunk. Basically the problem that we detected
is that create_worker could create two different threads to process
emails. The reason was that priv->is_running was not set until the
thread started to run, so it could happen that there were two calls to
create_worker that could start two different threads accessing the same
data without any locking to protect things like priv->cancel
CamelOperation for example.
BTW the attached patch includes also the patch I sent before in an email
with subject "Patch: change in send queue behaviour when notifying the
user" because it was not committed before, I completely forgot it :)
Basically the modifications for this issue are the ones related to
priv->is_running and the new priv->running_lock
Br
Index: libtinymail-camel/tny-camel-send-queue-priv.h
===================================================================
--- libtinymail-camel/tny-camel-send-queue-priv.h (revision 3813)
+++ libtinymail-camel/tny-camel-send-queue-priv.h (working copy)
@@ -36,6 +36,7 @@
gboolean do_continue, is_running;
gboolean observer_attached;
gint pending_send_notifies;
+ GStaticMutex *running_lock;
};
#endif
Index: libtinymail-camel/tny-camel-send-queue.c
===================================================================
--- libtinymail-camel/tny-camel-send-queue.c (revision 3813)
+++ libtinymail-camel/tny-camel-send-queue.c (working copy)
@@ -68,6 +68,9 @@
TnyHeader *header;
gint i, total;
guint signal_id;
+ GCond *condition;
+ GMutex *mutex;
+ gboolean had_callback;
} ControlInfo;
@@ -308,6 +311,13 @@
if (apriv)
tny_lockable_unlock (apriv->session->priv->ui_lock);
+ if (info->mutex) {
+ g_mutex_lock (info->mutex);
+ g_cond_broadcast (info->condition);
+ info->had_callback = TRUE;
+ g_mutex_unlock (info->mutex);
+ }
+
return FALSE;
}
@@ -567,6 +577,32 @@
g_slice_free (SyncSync, info);
}
+static void
+wait_for_queue_start_notification (TnySendQueue *self)
+{
+ ControlInfo *info = g_slice_new0 (ControlInfo);
+
+ info->mutex = g_mutex_new ();
+ info->condition = g_cond_new ();
+ info->had_callback = FALSE;
+
+ info->self = g_object_ref (self);
+ info->signal_id = TNY_SEND_QUEUE_START;
+
+ g_idle_add (emit_queue_control_signals_on_mainloop, info);
+
+ g_mutex_lock (info->mutex);
+ if (!info->had_callback)
+ g_cond_wait (info->condition, info->mutex);
+ g_mutex_unlock (info->mutex);
+
+ g_mutex_free (info->mutex);
+ g_cond_free (info->condition);
+
+ g_slice_free (GetHeadersSync, info);
+}
+
+
static gpointer
thread_main (gpointer data)
{
@@ -578,7 +614,9 @@
TnyDevice *device = info->device;
GHashTable *failed_headers = NULL;
- priv->is_running = TRUE;
+ /* Wait here until the user receives the queue-start notification */
+ wait_for_queue_start_notification (self);
+
list = tny_simple_list_new ();
g_static_rec_mutex_lock (priv->todo_lock);
@@ -621,7 +659,6 @@
if (tny_device_is_online (device))
get_headers_sync (info->outbox, headers, TRUE, &ferror);
else {
- priv->is_running = FALSE;
g_static_rec_mutex_unlock (priv->todo_lock);
g_static_rec_mutex_unlock (priv->sending_lock);
goto errorhandler;
@@ -629,7 +666,6 @@
if (ferror != NULL)
{
- priv->is_running = FALSE;
emit_error (self, NULL, NULL, ferror, i, priv->total);
g_error_free (ferror);
g_object_unref (headers);
@@ -676,7 +712,6 @@
if (length <= 0)
{
- priv->is_running = FALSE;
g_object_unref (headers);
g_static_rec_mutex_unlock (priv->todo_lock);
g_static_rec_mutex_unlock (priv->sending_lock);
@@ -765,7 +800,6 @@
g_object_unref (header);
} else
{
- priv->is_running = FALSE;
/* Not good, or we just went offline, let's just kill this thread */
length = 0;
if (header && G_IS_OBJECT (header))
@@ -783,8 +817,6 @@
errorhandler:
- priv->is_running = FALSE;
-
if (failed_headers)
g_hash_table_destroy (failed_headers);
@@ -814,6 +846,10 @@
g_slice_free (MainThreadInfo, info);
+ g_static_mutex_lock (priv->running_lock);
+ priv->is_running = FALSE;
+ g_static_mutex_unlock (priv->running_lock);
+
/* Emit the queue-stop signal */
emit_queue_control_signals (self, TNY_SEND_QUEUE_STOP);
@@ -827,6 +863,7 @@
{
TnyCamelSendQueuePriv *priv = TNY_CAMEL_SEND_QUEUE_GET_PRIVATE (self);
+ g_static_mutex_lock (priv->running_lock);
if (!priv->is_running && priv->trans_account && TNY_IS_TRANSPORT_ACCOUNT (priv->trans_account))
{
TnyCamelAccountPriv *apriv = TNY_CAMEL_ACCOUNT_GET_PRIVATE (priv->trans_account);
@@ -871,12 +908,19 @@
_tny_camel_folder_reason (spriv);
}
- emit_queue_control_signals (self, TNY_SEND_QUEUE_START);
-
+ priv->is_running = TRUE;
priv->thread = g_thread_create (thread_main, info, FALSE, NULL);
if (priv->thread == NULL) {
- emit_queue_control_signals (self, TNY_SEND_QUEUE_STOP);
+ GError *error;
+
+ priv->is_running = FALSE;
+
+ g_set_error (err, TNY_ERROR_DOMAIN,
+ TNY_SYSTEM_ERROR_UNKNOWN,
+ "Couldn't start thread for send queue");
+ emit_error (self, NULL, NULL, error, 0, 0);
+
if (TNY_IS_CAMEL_FOLDER (info->outbox)) {
TnyCamelFolderPriv *opriv = TNY_CAMEL_FOLDER_GET_PRIVATE (info->outbox);
_tny_camel_folder_unreason (opriv);
@@ -888,6 +932,7 @@
}
}
}
+ g_static_mutex_unlock (priv->running_lock);
return;
}
@@ -909,7 +954,9 @@
g_static_rec_mutex_lock (priv->sending_lock);
+ g_static_mutex_lock (priv->running_lock);
priv->is_running = FALSE;
+ g_static_mutex_unlock (priv->running_lock);
outbox = tny_send_queue_get_outbox (self);
@@ -991,7 +1038,7 @@
if (!err)
priv->total++;
- if (!err & !priv->is_running)
+ if (!err)
create_worker (info->self, &new_err);
/* Call user callback after msg has beed added to OUTBOX, waiting to be sent*/
@@ -1270,6 +1317,9 @@
g_free (priv->sending_lock);
priv->sending_lock = NULL;
+ g_free (priv->running_lock);
+ priv->running_lock = NULL;
+
g_object_unref (priv->trans_account);
(*parent_class->finalize) (object);
@@ -1472,6 +1522,8 @@
g_static_rec_mutex_init (priv->todo_lock);
priv->sending_lock = g_new0 (GStaticRecMutex, 1);
g_static_rec_mutex_init (priv->sending_lock);
+ priv->running_lock = g_new0 (GStaticMutex, 1);
+ g_static_mutex_init (priv->running_lock);
priv->is_running = FALSE;
priv->thread = NULL;
Index: ChangeLog
===================================================================
--- ChangeLog (revision 3813)
+++ ChangeLog (working copy)
@@ -1,7 +1,11 @@
2008-11-19 Sergio Villar Senin <svillar igalia com>
+ * libtinymail-camel/tny-camel-send-queue-priv.h
+ * libtinymail-camel/tny-camel-send-queue.c: added some locking to
+ prevent two send queue threads running at the same time.
+
* libtinymail-camel/tny-camel-mime-part.c
- (tny_camel_mime_part_decode_to_stream_default):
+ (tny_camel_mime_part_decode_to_stream_default):
adds a reference to protect wrapper when decoding to a stream
2008-11-19 Jose Dapena Paz <jdapena igalia com>
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]