Proposal for reverting the providers shutdown implementation

[Sergio Villar Senin <svillar igalia com>]

Hi,

long time ago, when Modest had a lot of problems when running in the
background we did a lot of things to try to ensure that all tinymail
threads were finished before exiting Modest. One of this things was the
implementation of a shutdown mechanism that waited for threads until all
of them finished.

Thing is that I believe that it's causing some deadlocks in POP code.
This provokes that for example, Modest starts to eat all the device
memory (N810 for example) available when running automatic updates in
the background, because everytime the autoupdate happens tinymail
creates a joinable thread that is never joined due to the deadlock I
mentioned.

What I propose, once we identified the problem of the automatic updates
in the past (which was an "invalid" call to PR_close IIRC) I  propose to
remove the shutdown mechanism code.

Br

PS: changes happened between revisions 3710 and 3711. The attached patch
is not a pure revert because Dape's commit included some refcount fixes
that should be kept.

Index: libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h	(revision 3710)
@@ -53,7 +53,6 @@
 	GStaticRecMutex *eng_lock, *uidl_lock;
 	gpointer book;
 	guint login_delay;
-	GThread *login_delay_thread;
 
 	GPtrArray *uids;
 	GHashTable *uids_uid;	/* messageinfo by uid */
@@ -79,7 +78,6 @@
 CamelType camel_pop3_store_get_type (void);
 
 void camel_pop3_store_destroy_lists (CamelPOP3Store *pop3_store);
-void camel_pop3_store_kill_threads (void);
 
 G_END_DECLS
 
Index: libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-provider.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-provider.c	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-provider.c	(revision 3710)
@@ -89,12 +89,6 @@
 	TRUE
 };
 
-static void
-pop3_shutdown (CamelProvider *provider)
-{
-	camel_pop3_store_kill_threads ();
-}
-
 void
 camel_provider_module_init(void)
 {
@@ -111,7 +105,6 @@
 	pop3_provider.authtypes = g_list_prepend(pop3_provider.authtypes, &camel_pop3_apop_authtype);
 	pop3_provider.authtypes = g_list_prepend(pop3_provider.authtypes, &camel_pop3_password_authtype);
 	pop3_provider.translation_domain = GETTEXT_PACKAGE;
-	pop3_provider.shutdown = pop3_shutdown;
 
 	camel_provider_register(&pop3_provider);
 }
Index: libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.c	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.c	(revision 3710)
@@ -83,9 +83,6 @@
 #define _(o) o
 
 static CamelStoreClass *parent_class = NULL;
-static GMutex *wait_for_login_mutex = NULL;
-static GCond *wait_for_login_cond = NULL;
-static GList *wait_for_login_threads = NULL;
 
 static void finalize (CamelObject *object);
 
@@ -407,18 +403,11 @@
 		login_delay = store->engine->login_delay;
 	g_static_rec_mutex_unlock (store->eng_lock);
 
-	g_mutex_lock (wait_for_login_mutex);
 	while (!killed) {
-		GTimeVal tv_delay = {0, 0};
 
-		tv_delay.tv_sec = login_delay;
+		sleep (login_delay);
 
-		g_cond_timed_wait (wait_for_login_cond, wait_for_login_mutex, &tv_delay);
 
-		if (!store->engine) {
-			break;
-		}
-
 		if (!store->is_refreshing) {
 			CamelException dex = CAMEL_EXCEPTION_INITIALISER;
 			g_static_rec_mutex_lock (store->uidl_lock);
@@ -433,8 +422,6 @@
 	}
 
 	camel_object_unref (store);
-	wait_for_login_threads = g_list_append (wait_for_login_threads, store->login_delay_thread);
-	g_mutex_unlock (wait_for_login_mutex);
 	return NULL;
 }
 
@@ -933,11 +920,8 @@
 static void 
 camel_pop3_store_prepare (CamelStore *store)
 {
-	CamelPOP3Store *pstore = (CamelPOP3Store *) store;
 	camel_object_ref (store);
-	g_mutex_lock (wait_for_login_mutex);
-	pstore->login_delay_thread = g_thread_create (wait_for_login_delay, store, TRUE, NULL);
-	g_mutex_unlock (wait_for_login_mutex);
+	g_thread_create (wait_for_login_delay, store, FALSE, NULL);
 }
 
 static gboolean
@@ -1213,22 +1199,6 @@
 
 }
 
-void
-camel_pop3_store_kill_threads (void)
-{
-	GThread *thread_to_join;
-	g_mutex_lock (wait_for_login_mutex);
-	while (wait_for_login_threads) {
-		thread_to_join = wait_for_login_threads->data;
-		g_cond_broadcast (wait_for_login_cond);
-		g_mutex_unlock (wait_for_login_mutex);
-		g_thread_join (thread_to_join);
-		g_mutex_lock (wait_for_login_mutex);
-		wait_for_login_threads = g_list_remove (wait_for_login_threads, thread_to_join);
-	}
-	g_mutex_unlock (wait_for_login_mutex);
-}
-
 static void
 camel_pop3_store_class_init (CamelPOP3StoreClass *camel_pop3_store_class)
 {
@@ -1240,9 +1210,6 @@
 		CAMEL_DISCO_STORE_CLASS (camel_pop3_store_class);
 
 	parent_class = CAMEL_STORE_CLASS (camel_type_get_global_classfuncs (camel_disco_store_get_type ()));
-	wait_for_login_mutex = g_mutex_new ();
-	wait_for_login_cond = g_cond_new ();
-	wait_for_login_threads = NULL;
 
 	/* virtual method overload */
 	camel_service_class->construct = pop3_construct;
@@ -1288,7 +1255,6 @@
 	g_static_rec_mutex_init (store->eng_lock);
 	store->uidl_lock = g_new0 (GStaticRecMutex, 1);
 	g_static_rec_mutex_init (store->uidl_lock);
-	store->login_delay_thread = NULL;
 
 	return;
 }
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c	(revision 3710)
@@ -113,7 +113,6 @@
 	imap_provider.authtypes = camel_sasl_authtype_list (FALSE);
 	imap_provider.authtypes = g_list_prepend (imap_provider.authtypes, &camel_imap_password_authtype);
 	imap_provider.translation_domain = GETTEXT_PACKAGE;
-	imap_provider.shutdown = NULL;
 
 	camel_provider_register(&imap_provider);
 }
Index: libtinymail-camel/camel-lite/camel/camel-provider.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-provider.c	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/camel-provider.c	(revision 3710)
@@ -269,26 +269,6 @@
 	UNLOCK();
 }
 
-void camel_provider_shutdown (CamelProvider *provider)
-{
-	if (provider->shutdown) {
-		provider->shutdown ((CamelObject *) provider);
-	}
-}
-
-void camel_provider_shutdown_all (void)
-{
-	GList *providers_list;
-	GList *node;
-
-	providers_list = camel_provider_list (FALSE);
-	for (node = providers_list; node != NULL; node = g_list_next (node)) {
-		CamelProvider *provider = (CamelProvider *) node->data;
-		camel_provider_shutdown (provider);
-	}
-	g_list_free (providers_list);
-}
-
 static gint
 provider_compare (gconstpointer a, gconstpointer b)
 {
Index: libtinymail-camel/camel-lite/camel/camel-provider.h
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-provider.h	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/camel-provider.h	(revision 3710)
@@ -144,9 +144,7 @@
 #define CAMEL_PROVIDER_CONF_DEFAULT_PATH      { CAMEL_PROVIDER_CONF_ENTRY, "path", NULL, N_("_Path:"), "" }
 
 typedef int (*CamelProviderAutoDetectFunc) (CamelURL *url, GHashTable **auto_detected, CamelException *ex);
-typedef void (*CamelProviderShutdownFunc) (CamelObject *provider);
 
-
 typedef struct {
 	/* Provider name used in CamelURLs. */
 	char *protocol;
@@ -210,10 +208,6 @@
 	 */
 	const char *license_file;
 
-	/* Shutdown function. Will be called in camel_shutdown.
-	 */
-	CamelProviderShutdownFunc shutdown;
-
 	/* Private to the provider */
 	void *priv;
 } CamelProvider;
@@ -230,8 +224,6 @@
 
 void camel_provider_load(const char *path, CamelException *ex);
 void camel_provider_register(CamelProvider *provider);
-void camel_provider_shutdown (CamelProvider *provider);
-void camel_provider_shutdown_all (void);
 GList *camel_provider_list(gboolean load);
 CamelProvider *camel_provider_get(const char *url_string, CamelException *ex);
 
Index: libtinymail-camel/camel-lite/camel/camel.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel.c	(revision 3711)
+++ libtinymail-camel/camel-lite/camel/camel.c	(revision 3710)
@@ -54,8 +54,6 @@
 	if (!initialised)
 		return;
 
-	camel_provider_shutdown_all ();
-
 	initialised = FALSE;
 	certdb = camel_certdb_get_default ();
 	if (certdb) {
Index: libtinymail-camel/tny-camel-pop-store-account.c
===================================================================
--- libtinymail-camel/tny-camel-pop-store-account.c	(revision 3711)
+++ libtinymail-camel/tny-camel-pop-store-account.c	(revision 3710)
@@ -95,13 +95,9 @@
 {
 	if (self && TNY_IS_CAMEL_STORE_ACCOUNT (self)) {
 		TnyCamelStoreAccountPriv *priv = TNY_CAMEL_STORE_ACCOUNT_GET_PRIVATE (self);
-		TnyCamelPopStoreAccountPriv *ppriv = TNY_CAMEL_POP_STORE_ACCOUNT_GET_PRIVATE (self);
 		g_static_rec_mutex_lock (priv->factory_lock);
 		priv->managed_folders = g_list_remove_all (priv->managed_folders, folder);
 		g_static_rec_mutex_unlock (priv->factory_lock);
-		if (ppriv->inbox == folder) {
-			ppriv->inbox = NULL;
-		}
 	}
 }
 
@@ -119,13 +115,11 @@
 		ppriv->inbox = TNY_FOLDER (_tny_camel_pop_folder_new ());
 		g_object_weak_ref (G_OBJECT (ppriv->inbox), (GWeakNotify) notify_factory_del, self);
 		priv->managed_folders = g_list_prepend (priv->managed_folders, ppriv->inbox);
-	} else {
-		g_object_ref (ppriv->inbox);
 	}
 
 	g_static_rec_mutex_unlock (priv->factory_lock);
 
-	return (TnyFolder *) ppriv->inbox;
+	return (TnyFolder *) g_object_ref (ppriv->inbox);
 }
 
 /**
@@ -148,6 +142,9 @@
 {
 	TnyCamelPopStoreAccountPriv *priv = TNY_CAMEL_POP_STORE_ACCOUNT_GET_PRIVATE (object);
 
+	if (priv->inbox)
+		g_object_unref (priv->inbox);
+
 	g_mutex_free (priv->lock);
 
 	(*parent_class->finalize) (object);