Bug 677438 - [POP3] Crash on disconnect with ongoing message download

4 files changed, 149 insertions, 11 deletions

diff --git a/camel/providers/pop3/camel-pop3-engine.c b/camel/providers/pop3/camel-pop3-engine.c
index 0dd59c33e..376d1e046 100644
--- a/camel/providers/pop3/camel-pop3-engine.c
+++ b/camel/providers/pop3/camel-pop3-engine.c
@@ -66,6 +66,9 @@ pop3_engine_finalize (GObject *object)
 	g_list_free (engine->auth);
 	g_free (engine->apop);
 
+	g_mutex_clear (&engine->busy_lock);
+	g_cond_clear (&engine->busy_cond);
+
 	/* Chain up to parent's finalize() method. */
 	G_OBJECT_CLASS (camel_pop3_engine_parent_class)->finalize (object);
 }
@@ -86,6 +89,9 @@ camel_pop3_engine_init (CamelPOP3Engine *engine)
 	g_queue_init (&engine->active);
 	g_queue_init (&engine->queue);
 	g_queue_init (&engine->done);
+	g_mutex_init (&engine->busy_lock);
+	g_cond_init (&engine->busy_cond);
+	engine->is_busy = FALSE;
 	engine->state = CAMEL_POP3_ENGINE_DISCONNECT;
 }
 
@@ -240,6 +246,9 @@ get_capabilities (CamelPOP3Engine *pe,
 	g_return_val_if_fail (pe != NULL, FALSE);
 
 	if (!(pe->flags & CAMEL_POP3_ENGINE_DISABLE_EXTENSIONS)) {
+		if (!camel_pop3_engine_busy_lock (pe, cancellable, error))
+			return FALSE;
+
 		pc = camel_pop3_engine_command_new (pe, CAMEL_POP3_COMMAND_MULTI, cmd_capa, NULL, cancellable, &local_error, "CAPA\r\n");
 		while (camel_pop3_engine_iterate (pe, pc, cancellable, &local_error) > 0)
 			;
@@ -256,6 +265,8 @@ get_capabilities (CamelPOP3Engine *pe,
 
 			camel_pop3_engine_command_free (pe, pc);
 		}
+
+		camel_pop3_engine_busy_unlock (pe);
 	}
 
 	if (local_error) {
@@ -429,6 +440,73 @@ ioerror:
 	return -1;
 }
 
+static void
+camel_pop3_engine_wait_cancelled_cb (GCancellable *cancellable,
+				     gpointer user_data)
+{
+	CamelPOP3Engine *pe = user_data;
+
+	g_return_if_fail (CAMEL_IS_POP3_ENGINE (pe));
+
+	g_mutex_lock (&pe->busy_lock);
+	g_cond_broadcast (&pe->busy_cond);
+	g_mutex_unlock (&pe->busy_lock);
+}
+
+/* Returns whether received the busy lock; if TRUE, then release it
+   with camel_pop3_engine_busy_unlock() when done with it. */
+gboolean
+camel_pop3_engine_busy_lock (CamelPOP3Engine *pe,
+			     GCancellable *cancellable,
+			     GError **error)
+{
+	gulong handler_id = 0;
+	gboolean got_lock = FALSE;
+
+	g_return_val_if_fail (CAMEL_IS_POP3_ENGINE (pe), FALSE);
+
+	if (g_cancellable_set_error_if_cancelled (cancellable, error))
+		return FALSE;
+
+	if (cancellable)
+		handler_id = g_cancellable_connect (cancellable, G_CALLBACK (camel_pop3_engine_wait_cancelled_cb), pe, NULL);
+
+	g_mutex_lock (&pe->busy_lock);
+	while (pe->is_busy) {
+		if (g_cancellable_set_error_if_cancelled (cancellable, error))
+			break;
+
+		g_cond_wait (&pe->busy_cond, &pe->busy_lock);
+	}
+
+	if (!pe->is_busy && !g_cancellable_is_cancelled (cancellable)) {
+		pe->is_busy = TRUE;
+		got_lock = TRUE;
+	}
+
+	g_mutex_unlock (&pe->busy_lock);
+
+	if (handler_id)
+		g_cancellable_disconnect (cancellable, handler_id);
+
+	return got_lock;
+}
+
+void
+camel_pop3_engine_busy_unlock (CamelPOP3Engine *pe)
+{
+	g_return_if_fail (CAMEL_IS_POP3_ENGINE (pe));
+
+	g_mutex_lock (&pe->busy_lock);
+
+	g_warn_if_fail (pe->is_busy);
+	pe->is_busy = FALSE;
+
+	g_cond_broadcast (&pe->busy_cond);
+
+	g_mutex_unlock (&pe->busy_lock);
+}
+
 CamelPOP3Command *
 camel_pop3_engine_command_new (CamelPOP3Engine *pe,
                                guint32 flags,
diff --git a/camel/providers/pop3/camel-pop3-engine.h b/camel/providers/pop3/camel-pop3-engine.h
index a468da70b..c71a464da 100644
--- a/camel/providers/pop3/camel-pop3-engine.h
+++ b/camel/providers/pop3/camel-pop3-engine.h
@@ -131,6 +131,10 @@ struct _CamelPOP3Engine {
 	GQueue done;	/* list of done commands, awaiting free */
 
 	CamelPOP3Command *current; /* currently busy (downloading) response */
+
+	GMutex busy_lock;
+	GCond busy_cond;
+	gboolean is_busy;
 };
 
 struct _CamelPOP3EngineClass {
@@ -151,6 +155,10 @@ gint		camel_pop3_engine_iterate	(CamelPOP3Engine *pe,
 						 CamelPOP3Command *pc,
 						 GCancellable *cancellable,
 						 GError **error);
+gboolean	camel_pop3_engine_busy_lock	(CamelPOP3Engine *pe,
+						 GCancellable *cancellable,
+						 GError **error);
+void		camel_pop3_engine_busy_unlock	(CamelPOP3Engine *pe);
 CamelPOP3Command *
 		camel_pop3_engine_command_new	(CamelPOP3Engine *pe,
 						 guint32 flags,
diff --git a/camel/providers/pop3/camel-pop3-folder.c b/camel/providers/pop3/camel-pop3-folder.c
index 4c785e777..970662408 100644
--- a/camel/providers/pop3/camel-pop3-folder.c
+++ b/camel/providers/pop3/camel-pop3-folder.c
@@ -71,7 +71,7 @@ cmd_uidl (CamelPOP3Engine *pe,
 			if (sscanf ((gchar *) line, "%u %s", &id, uid) == 2) {
 				fi = g_hash_table_lookup (folder->uids_id, GINT_TO_POINTER (id));
 				if (fi) {
-					camel_operation_progress (NULL, (fi->index + 1) * 100 / folder->uids->len);
+					camel_operation_progress (cancellable, (fi->index + 1) * 100 / folder->uids->len);
 					fi->uid = g_strdup (uid);
 					g_hash_table_insert (folder->uids_fi, fi->uid, fi);
 				} else {
@@ -102,7 +102,7 @@ cmd_builduid (CamelPOP3Engine *pe,
 
 	/* TODO; somehow work out the limit and use that for proper progress reporting
 	 * We need a pointer to the folder perhaps? */
-	camel_operation_progress (NULL, fi->id);
+	/* camel_operation_progress (cancellable, fi->id); */
 
 	checksum = g_checksum_new (G_CHECKSUM_MD5);
 	mp = camel_mime_parser_new ();
@@ -202,7 +202,7 @@ cmd_tocache (CamelPOP3Engine *pe,
 		if (w > fi->size)
 			w = fi->size;
 		if (fi->size != 0)
-			camel_operation_progress (NULL, (w * 100) / fi->size);
+			camel_operation_progress (cancellable, (w * 100) / fi->size);
 	}
 
 	/* it all worked, output a '#' to say we're a-ok */
@@ -457,6 +457,9 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 
 	pop3_engine = camel_pop3_store_ref_engine (pop3_store);
 
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error))
+		goto fail;
+
 	/* If we have an oustanding retrieve message running, wait for that to complete
 	 * & then retrieve from cache, otherwise, start a new one, and similar */
 
@@ -472,6 +475,7 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 		if (i == -1) {
 			g_prefix_error (
 				error, _("Cannot get message %s: "), uid);
+			camel_pop3_engine_busy_unlock (pop3_engine);
 			goto fail;
 		}
 	}
@@ -581,6 +585,7 @@ pop3_folder_get_message_sync (CamelFolder *folder,
 		camel_medium_add_header (CAMEL_MEDIUM (message), "X-Evolution-POP3-UID", uid);
 	}
 done:
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&stream);
 fail:
 	g_clear_object (&pop3_engine);
@@ -615,11 +620,16 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 		return FALSE;
 	}
 
+	pop3_engine = camel_pop3_store_ref_engine (pop3_store);
+
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error)) {
+		g_clear_object (&pop3_engine);
+		return FALSE;
+	}
+
 	camel_operation_push_message (
 		cancellable, _("Retrieving POP summary"));
 
-	pop3_engine = camel_pop3_store_ref_engine (pop3_store);
-
 	/* Get rid of the old cache */
 	if (pop3_folder->uids) {
 		gint i;
@@ -736,6 +746,7 @@ pop3_folder_refresh_info_sync (CamelFolder *folder,
 	g_hash_table_destroy (pop3_folder->uids_id);
 	pop3_folder->uids_id = NULL;
 
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&pop3_engine);
 
 	camel_operation_pop_message (cancellable);
@@ -817,8 +828,18 @@ pop3_folder_synchronize_sync (CamelFolder *folder,
 	pop3_cache = camel_pop3_store_ref_cache (pop3_store);
 	pop3_engine = camel_pop3_store_ref_engine (pop3_store);
 
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error)) {
+			g_clear_object (&pop3_cache);
+			g_clear_object (&pop3_engine);
+
+			camel_operation_pop_message (cancellable);
+
+			return FALSE;
+	}
+
 	for (i = 0; i < pop3_folder->uids->len; i++) {
 		if (g_cancellable_set_error_if_cancelled (cancellable, error)) {
+			camel_pop3_engine_busy_unlock (pop3_engine);
 			g_clear_object (&pop3_cache);
 			g_clear_object (&pop3_engine);
 
@@ -851,6 +872,7 @@ pop3_folder_synchronize_sync (CamelFolder *folder,
 
 	for (i = 0; i < pop3_folder->uids->len; i++) {
 		if (g_cancellable_set_error_if_cancelled (cancellable, error)) {
+			camel_pop3_engine_busy_unlock (pop3_engine);
 			g_clear_object (&pop3_cache);
 			g_clear_object (&pop3_engine);
 
@@ -871,6 +893,7 @@ pop3_folder_synchronize_sync (CamelFolder *folder,
 			cancellable, (i + 1) * 100 / pop3_folder->uids->len);
 	}
 
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&pop3_cache);
 	g_clear_object (&pop3_engine);
 
@@ -1017,6 +1040,13 @@ camel_pop3_folder_delete_old (CamelFolder *folder,
 	pop3_cache = camel_pop3_store_ref_cache (pop3_store);
 	pop3_engine = camel_pop3_store_ref_engine (pop3_store);
 
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error)) {
+			g_clear_object (&pop3_cache);
+			g_clear_object (&pop3_engine);
+
+			return FALSE;
+	}
+
 	temp = time (&temp);
 
 	d (printf ("%s(%d): pop3_folder->uids->len=[%d]\n", __FILE__, __LINE__, pop3_folder->uids->len));
@@ -1025,6 +1055,7 @@ camel_pop3_folder_delete_old (CamelFolder *folder,
 		fi = pop3_folder->uids->pdata[i];
 
 		if (g_cancellable_set_error_if_cancelled (cancellable, error)) {
+			camel_pop3_engine_busy_unlock (pop3_engine);
 			g_clear_object (&pop3_cache);
 			g_clear_object (&pop3_engine);
 
@@ -1068,6 +1099,7 @@ camel_pop3_folder_delete_old (CamelFolder *folder,
 
 			if (day_lag > days_to_delete) {
 				if (g_cancellable_set_error_if_cancelled (cancellable, error)) {
+					camel_pop3_engine_busy_unlock (pop3_engine);
 					g_clear_object (&pop3_cache);
 					g_clear_object (&pop3_engine);
 
@@ -1100,6 +1132,7 @@ camel_pop3_folder_delete_old (CamelFolder *folder,
 
 	for (i = 0; i < pop3_folder->uids->len; i++) {
 		if (g_cancellable_set_error_if_cancelled (cancellable, error)) {
+			camel_pop3_engine_busy_unlock (pop3_engine);
 			g_clear_object (&pop3_cache);
 			g_clear_object (&pop3_engine);
 
@@ -1118,6 +1151,7 @@ camel_pop3_folder_delete_old (CamelFolder *folder,
 			cancellable, (i + 1) * 100 / pop3_folder->uids->len);
 	}
 
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&pop3_cache);
 	g_clear_object (&pop3_engine);
 
diff --git a/camel/providers/pop3/camel-pop3-store.c b/camel/providers/pop3/camel-pop3-store.c
index 25dd1ae10..42744c0e6 100644
--- a/camel/providers/pop3/camel-pop3-store.c
+++ b/camel/providers/pop3/camel-pop3-store.c
@@ -610,12 +610,15 @@ pop3_store_disconnect_sync (CamelService *service,
 		pop3_engine = camel_pop3_store_ref_engine (store);
 
 		if (pop3_engine) {
-			pc = camel_pop3_engine_command_new (
-				pop3_engine, 0, NULL, NULL,
-				cancellable, error, "QUIT\r\n");
-			while (camel_pop3_engine_iterate (pop3_engine, NULL, cancellable, NULL) > 0)
-				;
-			camel_pop3_engine_command_free (pop3_engine, pc);
+			if (camel_pop3_engine_busy_lock (pop3_engine, cancellable, NULL)) {
+				pc = camel_pop3_engine_command_new (
+					pop3_engine, 0, NULL, NULL,
+					cancellable, error, "QUIT\r\n");
+				while (camel_pop3_engine_iterate (pop3_engine, NULL, cancellable, NULL) > 0)
+					;
+				camel_pop3_engine_command_free (pop3_engine, pc);
+				camel_pop3_engine_busy_unlock (pop3_engine);
+			}
 
 			g_clear_object (&pop3_engine);
 		}
@@ -671,6 +674,14 @@ pop3_store_authenticate_sync (CamelService *service,
 		goto exit;
 	}
 
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error)) {
+		g_free (host);
+		g_free (user);
+		g_clear_object (&pop3_engine);
+
+		return CAMEL_AUTHENTICATION_ERROR;
+	}
+
 	if (mechanism == NULL) {
 		if (password == NULL) {
 			g_set_error_literal (
@@ -820,6 +831,7 @@ exit:
 	g_free (host);
 	g_free (user);
 
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&pop3_engine);
 
 	return result;
@@ -1094,6 +1106,11 @@ camel_pop3_store_expunge (CamelPOP3Store *store,
 
 	pop3_engine = camel_pop3_store_ref_engine (store);
 
+	if (!camel_pop3_engine_busy_lock (pop3_engine, cancellable, error)) {
+		g_clear_object (&pop3_engine);
+		return FALSE;
+	}
+
 	pc = camel_pop3_engine_command_new (
 		pop3_engine, 0, NULL, NULL, cancellable, error, "QUIT\r\n");
 
@@ -1102,6 +1119,7 @@ camel_pop3_store_expunge (CamelPOP3Store *store,
 
 	camel_pop3_engine_command_free (pop3_engine, pc);
 
+	camel_pop3_engine_busy_unlock (pop3_engine);
 	g_clear_object (&pop3_engine);
 
 	return TRUE;