Patch: some more leak fixes for pop3 provider



	Hi,

	This patch includes a small fix for a object reference leak (from pop3
store to pop3 folder), and a major change to kill properly the login
delay thread in pop3 store.

	The later consists on adding a shutdown handler to CamelProvider, so
that we can call it on shutting down camel. The implementation for pop3
kills the delay threads properly.

Changelog would be:
* libtinymail-camel/camel-lite/camel/camel-provider.[ch]:
        * Now providers offer a new shutdown handler, that will be 
	  called on shutting down camel.
* libtinymail-camel/camel-lite/camel/camel.c:
        * Use the providers shutdown handler on shutting down camel.
* libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-provider.c:
        * Implement new provider shutdown handler, to kill the login delay         
          threads on shutting down tinymail.
* libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.[ch]:
        * New method to kill the login delay thread. This is used to kill
          the thread on shutting down tinymail.
* libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c:
        * Set as NULL the default shutdown handler. 
* libtinymail-camel/tny-camel-pop-store-account.c:
        * Manage properly the internal inbox reference to avoid leaking it.

-- 
José Dapena Paz <jdapena igalia com>
Igalia
Index: libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h	(revision 3709)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.h	(working copy)
@@ -53,6 +53,7 @@
 	GStaticRecMutex *eng_lock, *uidl_lock;
 	gpointer book;
 	guint login_delay;
+	GThread *login_delay_thread;
 
 	GPtrArray *uids;
 	GHashTable *uids_uid;	/* messageinfo by uid */
@@ -78,6 +79,7 @@
 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 3709)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-provider.c	(working copy)
@@ -89,6 +89,12 @@
 	TRUE
 };
 
+static void
+pop3_shutdown (CamelProvider *provider)
+{
+	camel_pop3_store_kill_threads ();
+}
+
 void
 camel_provider_module_init(void)
 {
@@ -105,6 +111,7 @@
 	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 3709)
+++ libtinymail-camel/camel-lite/camel/providers/pop3/camel-pop3-store.c	(working copy)
@@ -83,6 +83,9 @@
 #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);
 
@@ -161,7 +164,7 @@
 	g_static_rec_mutex_lock (store->eng_lock);
 
 	if (store->engine == NULL) {
-		g_static_rec_mutex_lock (store->eng_lock);
+		g_static_rec_mutex_unlock (store->eng_lock);
 		return TRUE;
 	}
 
@@ -174,9 +177,10 @@
 		camel_pop3_engine_command_free(store->engine, pc);
 	}
 
+	g_static_rec_mutex_unlock (store->eng_lock);
+
 	g_timeout_add (20000, unref_it, store->engine);
 	store->engine = NULL;
-	g_static_rec_mutex_unlock (store->eng_lock);
 
 	/* camel_object_unref((CamelObject *)store->engine); */
 
@@ -403,11 +407,18 @@
 		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};
 
-		sleep (login_delay);
+		tv_delay.tv_sec = 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);
@@ -422,6 +433,8 @@
 	}
 
 	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;
 }
 
@@ -920,8 +933,11 @@
 static void 
 camel_pop3_store_prepare (CamelStore *store)
 {
+	CamelPOP3Store *pstore = (CamelPOP3Store *) store;
 	camel_object_ref (store);
-	g_thread_create (wait_for_login_delay, store, FALSE, NULL);
+	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);
 }
 
 static gboolean
@@ -1019,9 +1035,7 @@
 	}
 
 	g_timeout_add (20000, unref_it, store->engine);
-	/* camel_object_unref((CamelObject *)store->engine); */
 	store->engine = NULL;
-
 	//g_static_rec_mutex_unlock (store->eng_lock);
 
 	return TRUE;
@@ -1199,6 +1213,22 @@
 
 }
 
+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)
 {
@@ -1210,6 +1240,9 @@
 		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;
@@ -1255,6 +1288,7 @@
 	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/camel-provider.c
===================================================================
--- libtinymail-camel/camel-lite/camel/camel-provider.c	(revision 3709)
+++ libtinymail-camel/camel-lite/camel/camel-provider.c	(working copy)
@@ -269,6 +269,26 @@
 	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 3709)
+++ libtinymail-camel/camel-lite/camel/camel-provider.h	(working copy)
@@ -144,7 +144,9 @@
 #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;
@@ -208,6 +210,10 @@
 	 */
 	const char *license_file;
 
+	/* Shutdown function. Will be called in camel_shutdown.
+	 */
+	CamelProviderShutdownFunc shutdown;
+
 	/* Private to the provider */
 	void *priv;
 } CamelProvider;
@@ -224,6 +230,8 @@
 
 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 3709)
+++ libtinymail-camel/camel-lite/camel/camel.c	(working copy)
@@ -54,6 +54,8 @@
 	if (!initialised)
 		return;
 
+	camel_provider_shutdown_all ();
+
 	initialised = FALSE;
 	certdb = camel_certdb_get_default ();
 	if (certdb) {
Index: libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c
===================================================================
--- libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c	(revision 3709)
+++ libtinymail-camel/camel-lite/camel/providers/imap/camel-imap-provider.c	(working copy)
@@ -113,6 +113,7 @@
 	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/tny-camel-pop-store-account.c
===================================================================
--- libtinymail-camel/tny-camel-pop-store-account.c	(revision 3709)
+++ libtinymail-camel/tny-camel-pop-store-account.c	(working copy)
@@ -95,9 +95,13 @@
 {
 	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;
+		}
 	}
 }
 
@@ -115,11 +119,13 @@
 		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 *) g_object_ref (ppriv->inbox);
+	return (TnyFolder *) ppriv->inbox;
 }
 
 /**
@@ -142,9 +148,6 @@
 {
 	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);


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]