commit 6dfa9e5718df60aea5a12b5a4ae28da28a29f54e
parent ff2f663ebca80be4991c71be724d5a29ef5a3bdf
Author: Colin Leroy <colin@colino.net>
Date: Thu, 18 Dec 2014 10:18:48 +0100
Merge remote-tracking branch 'jakub-kicinski/for-master'
Diffstat:
5 files changed, 96 insertions(+), 69 deletions(-)
diff --git a/src/etpan/etpan-thread-manager.c b/src/etpan/etpan-thread-manager.c
@@ -187,19 +187,8 @@ struct etpan_thread_op * etpan_thread_op_new(void)
op = malloc(sizeof(* op));
if (op == NULL)
goto err;
-
- op->thread = NULL;
- op->run = NULL;
- op->callback = NULL;
- op->callback_data = NULL;
- op->callback_called = 0;
- op->cancellable = 0;
- op->cancelled = 0;
- op->param = NULL;
- op->result = NULL;
- op->finished = 0;
- op->imap = NULL;
- op->nntp = NULL;
+
+ memset(op, 0, sizeof(* op));
r = pthread_mutex_init(&op->lock, NULL);
if (r != 0)
diff --git a/src/etpan/imap-thread.c b/src/etpan/imap-thread.c
@@ -59,26 +59,6 @@ static chash * session_hash = NULL;
static guint thread_manager_signal = 0;
static GIOChannel * io_channel = NULL;
-static void delete_imap(Folder *folder, mailimap *imap)
-{
- chashdatum key;
-
- key.data = &folder;
- key.len = sizeof(folder);
- chash_delete(session_hash, &key, NULL);
-
- key.data = &imap;
- key.len = sizeof(imap);
- chash_delete(courier_workaround_hash, &key, NULL);
- if (imap && imap->imap_stream) {
- /* we don't want libetpan to logout */
- mailstream_close(imap->imap_stream);
- imap->imap_stream = NULL;
- }
- debug_print("removing mailimap %p\n", imap);
- mailimap_free(imap);
-}
-
static gboolean thread_manager_event(GIOChannel * source,
GIOCondition condition,
gpointer data)
@@ -416,38 +396,47 @@ static void generic_cb(int cancelled, void * result, void * callback_data)
op->finished = 1;
}
-static void threaded_run(Folder * folder, void * param, void * result,
- void (* func)(struct etpan_thread_op * ))
+/* Please do *not* blindly use imap pointers after this function returns,
+ * someone may have deleted it while this function was waiting for completion.
+ * Check return value to see if imap is still valid.
+ * Run get_imap(folder) again to get a fresh and valid pointer.
+ */
+static int threaded_run(Folder * folder, void * param, void * result,
+ void (* func)(struct etpan_thread_op * ))
{
struct etpan_thread_op * op;
struct etpan_thread * thread;
+ struct mailimap * imap = get_imap(folder);
imap_folder_ref(folder);
op = etpan_thread_op_new();
- op->imap = get_imap(folder);
+ op->imap = imap;
op->param = param;
op->result = result;
- op->cancellable = 0;
op->run = func;
op->callback = generic_cb;
op->callback_data = op;
- op->cleanup = NULL;
-
- op->finished = 0;
-
+
thread = get_thread(folder);
etpan_thread_op_schedule(thread, op);
while (!op->finished) {
gtk_main_iteration();
}
-
+
etpan_thread_op_free(op);
imap_folder_unref(folder);
+
+ if (imap != get_imap(folder)) {
+ g_warning("returning from operation on a stale imap %p", imap);
+ return 1;
+ }
+
+ return 0;
}
@@ -471,6 +460,55 @@ struct connect_result {
} \
}
+
+static void delete_imap_run(struct etpan_thread_op * op)
+{
+ mailimap * imap = op->imap;
+
+ /* we don't want libetpan to logout */
+ if (imap->imap_stream) {
+ mailstream_close(imap->imap_stream);
+ imap->imap_stream = NULL;
+ }
+
+ mailimap_free(imap);
+}
+
+static void threaded_delete_imap(Folder *folder, mailimap *imap)
+{
+ struct etpan_thread_op * op;
+
+ /* No need to wait for completion, threaded_run() won't work here. */
+ op = etpan_thread_op_new();
+ op->imap = imap;
+ op->run = delete_imap_run;
+ op->cleanup = etpan_thread_op_free;
+
+ etpan_thread_op_schedule(get_thread(folder), op);
+
+ debug_print("threaded delete imap posted\n");
+}
+
+static void delete_imap(Folder *folder, mailimap *imap)
+{
+ chashdatum key;
+
+ key.data = &folder;
+ key.len = sizeof(folder);
+ chash_delete(session_hash, &key, NULL);
+
+ if (!imap)
+ return;
+ key.data = &imap;
+ key.len = sizeof(imap);
+ chash_delete(courier_workaround_hash, &key, NULL);
+ /* We can't just free imap here as there may be ops on it pending
+ * in the thread. Posting freeing as an op will synchronize against
+ * existing jobs and as imap is already removed from session_hash
+ * we are sure no new ops can be posted. */
+ threaded_delete_imap(folder, imap);
+}
+
static void connect_run(struct etpan_thread_op * op)
{
int r;
@@ -574,7 +612,8 @@ int imap_threaded_connect_ssl(Folder * folder, const char * server, int port)
accept_if_valid = folder->account->ssl_certs_auto_accept;
refresh_resolvers();
- threaded_run(folder, ¶m, &result, connect_ssl_run);
+ if (threaded_run(folder, ¶m, &result, connect_ssl_run))
+ return MAILIMAP_ERROR_INVAL;
if ((result.error == MAILIMAP_NO_ERROR_AUTHENTICATED ||
result.error == MAILIMAP_NO_ERROR_NON_AUTHENTICATED) && !etpan_skip_ssl_cert_check) {
@@ -615,7 +654,7 @@ static void capability_run(struct etpan_thread_op * op)
}
-struct mailimap_capability_data * imap_threaded_capability(Folder *folder, int *ok)
+int imap_threaded_capability(Folder *folder, struct mailimap_capability_data ** caps)
{
struct capa_param param;
struct capa_result result;
@@ -629,10 +668,10 @@ struct mailimap_capability_data * imap_threaded_capability(Folder *folder, int *
debug_print("capa %d\n", result.error);
- if (ok)
- *ok = result.error;
+ if (result.error == MAILIMAP_NO_ERROR)
+ *caps = result.caps;
- return result.caps;
+ return result.error;
}
@@ -674,13 +713,11 @@ void imap_threaded_disconnect(Folder * folder)
param.imap = imap;
- threaded_run(folder, ¶m, &result, disconnect_run);
-
- if (imap == get_imap(folder)) {
+ if (threaded_run(folder, ¶m, &result, disconnect_run)) {
+ debug_print("imap already deleted %p\n", imap);
+ } else {
debug_print("deleting old imap %p\n", imap);
delete_imap(folder, imap);
- } else {
- debug_print("imap already deleted %p\n", imap);
}
debug_print("disconnect ok\n");
@@ -1025,8 +1062,9 @@ int imap_threaded_noop(Folder * folder, unsigned int * p_exists,
imap = get_imap(folder);
param.imap = imap;
- threaded_run(folder, ¶m, &result, noop_run);
-
+ if (threaded_run(folder, ¶m, &result, noop_run))
+ return MAILIMAP_ERROR_INVAL;
+
if (result.error == 0 && imap && imap->imap_selection_info != NULL) {
* p_exists = imap->imap_selection_info->sel_exists;
* p_recent = imap->imap_selection_info->sel_recent;
@@ -1115,7 +1153,8 @@ int imap_threaded_starttls(Folder * folder, const gchar *host, int port)
if (folder->account)
accept_if_valid = folder->account->ssl_certs_auto_accept;
- threaded_run(folder, ¶m, &result, starttls_run);
+ if (threaded_run(folder, ¶m, &result, starttls_run))
+ return MAILIMAP_ERROR_INVAL;
debug_print("imap starttls - end\n");
@@ -1309,8 +1348,9 @@ int imap_threaded_select(Folder * folder, const char * mb,
param.imap = imap;
param.mb = mb;
- threaded_run(folder, ¶m, &result, select_run);
-
+ if (threaded_run(folder, ¶m, &result, select_run))
+ return MAILIMAP_ERROR_INVAL;
+
if (result.error != MAILIMAP_NO_ERROR)
return result.error;
@@ -1460,8 +1500,9 @@ int imap_threaded_examine(Folder * folder, const char * mb,
param.imap = imap;
param.mb = mb;
- threaded_run(folder, ¶m, &result, examine_run);
-
+ if (threaded_run(folder, ¶m, &result, examine_run))
+ return MAILIMAP_ERROR_INVAL;
+
if (result.error != MAILIMAP_NO_ERROR)
return result.error;
@@ -2903,8 +2944,9 @@ int imap_threaded_fetch_env(Folder * folder, struct mailimap_set * set,
param.imap = imap;
param.set = set;
- threaded_run(folder, ¶m, &result, fetch_env_run);
-
+ if (threaded_run(folder, ¶m, &result, fetch_env_run))
+ return MAILIMAP_ERROR_INVAL;
+
if (result.error != MAILIMAP_NO_ERROR) {
chashdatum key;
chashdatum value;
diff --git a/src/etpan/imap-thread.h b/src/etpan/imap-thread.h
@@ -47,7 +47,7 @@ void imap_done(Folder * folder);
int imap_threaded_connect(Folder * folder, const char * server, int port);
int imap_threaded_connect_ssl(Folder * folder, const char * server, int port);
-struct mailimap_capability_data * imap_threaded_capability(Folder *folder, int *ok);
+int imap_threaded_capability(Folder *folder, struct mailimap_capability_data ** caps);
#ifndef G_OS_WIN32
int imap_threaded_connect_cmd(Folder * folder, const char * command,
diff --git a/src/etpan/nntp-thread.c b/src/etpan/nntp-thread.c
@@ -277,14 +277,10 @@ static void threaded_run(Folder * folder, void * param, void * result,
op->nntp = get_nntp(folder);
op->param = param;
op->result = result;
-
- op->cancellable = 0;
+
op->run = func;
op->callback = generic_cb;
op->callback_data = op;
- op->cleanup = NULL;
-
- op->finished = 0;
previous_stream_logger = mailstream_logger;
mailstream_logger = nntp_logger;
diff --git a/src/imap.c b/src/imap.c
@@ -835,12 +835,12 @@ static int imap_get_capabilities(IMAPSession *session)
{
struct mailimap_capability_data *capabilities = NULL;
clistiter *cur;
- int result = -1;
+ int result;
if (session->capability != NULL)
return MAILIMAP_NO_ERROR;
- capabilities = imap_threaded_capability(session->folder, &result);
+ result = imap_threaded_capability(session->folder, &capabilities);
if (result != MAILIMAP_NO_ERROR) {
return result;
