Patch: some more leak fixes for pop3 provider
- From: José Dapena Paz <jdapena igalia com>
- To: tinymail-devel-list <tinymail-devel-list gnome org>
- Subject: Patch: some more leak fixes for pop3 provider
- Date: Tue, 01 Jul 2008 15:32:13 +0200
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]