[Commits] [SCM] claws branch, master, updated. 3.11.1-47-g6dfa9e5
Colin
colin at claws-mail.org
Thu Dec 18 10:28:26 CET 2014
The branch, master has been updated
via 6dfa9e5718df60aea5a12b5a4ae28da28a29f54e (commit)
via 0d02a35cc1f0798e1be6861433a7e78602f0b90f (commit)
via 7d5e6ca5a32586434e0ecaa2ce16dc1ce7419b91 (commit)
via 6d36a13352bf5fa16a4adbdba4a70721f6617f84 (commit)
from ff2f663ebca80be4991c71be724d5a29ef5a3bdf (commit)
Summary of changes:
src/etpan/etpan-thread-manager.c | 15 +----
src/etpan/imap-thread.c | 138 +++++++++++++++++++++++++-------------
src/etpan/imap-thread.h | 2 +-
src/etpan/nntp-thread.c | 6 +-
src/imap.c | 4 +-
5 files changed, 96 insertions(+), 69 deletions(-)
- Log -----------------------------------------------------------------
commit 6dfa9e5718df60aea5a12b5a4ae28da28a29f54e
Merge: ff2f663 0d02a35
Author: Colin Leroy <colin at colino.net>
Date: Thu Dec 18 10:18:48 2014 +0100
Merge remote-tracking branch 'jakub-kicinski/for-master'
commit 0d02a35cc1f0798e1be6861433a7e78602f0b90f
Author: Jakub Kicinski <kubakici at wp.pl>
Date: Fri Dec 12 10:00:51 2014 +0100
Use memset to initialize struct etpan_thread_op
also remove duplicated zeroing.
diff --git a/src/etpan/etpan-thread-manager.c b/src/etpan/etpan-thread-manager.c
index f72e8c7..f73dbfe 100644
--- 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
index 4998e33..5f79cdb 100644
--- a/src/etpan/imap-thread.c
+++ b/src/etpan/imap-thread.c
@@ -416,14 +416,10 @@ static int threaded_run(Folder * folder, void * param, void * result,
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);
diff --git a/src/etpan/nntp-thread.c b/src/etpan/nntp-thread.c
index ac5d210..c208028 100644
--- 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;
commit 7d5e6ca5a32586434e0ecaa2ce16dc1ce7419b91
Author: Jakub Kicinski <kubakici at wp.pl>
Date: Fri Dec 12 09:48:57 2014 +0100
Synchronize mailimap deletion against async operations
Right now main thread can free mailimap while its background thread
is still performing operations on it. Fix it by moving deletion to
the worker thread and making sure nobody uses stale pointers.
diff --git a/src/etpan/imap-thread.c b/src/etpan/imap-thread.c
index 66514ff..4998e33 100644
--- 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,17 +396,23 @@ 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;
@@ -444,10 +430,17 @@ static void threaded_run(Folder * folder, void * param, void * result,
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 +464,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 +616,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) {
@@ -674,13 +717,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 +1066,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 +1157,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 +1352,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 +1504,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 +2948,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;
commit 6d36a13352bf5fa16a4adbdba4a70721f6617f84
Author: Jakub Kicinski <kubakici at wp.pl>
Date: Fri Dec 12 09:17:34 2014 +0100
Swap return value and argument of imap_threaded_capability()
imap_threaded_capability() is the only imap_threaded_*() function
which does not return status, instead status is written to a pointer
passed as argument and function returns retrieved capabilities. To
unify behaviour of all imap_threaded_*() functions make it return
status and pass capabilities through an out argument.
diff --git a/src/etpan/imap-thread.c b/src/etpan/imap-thread.c
index 229f29e..66514ff 100644
--- a/src/etpan/imap-thread.c
+++ b/src/etpan/imap-thread.c
@@ -615,7 +615,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 +629,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;
}
diff --git a/src/etpan/imap-thread.h b/src/etpan/imap-thread.h
index fd61b5d..30bce70 100644
--- 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/imap.c b/src/imap.c
index 32ecd38..35fb47f 100644
--- 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;
-----------------------------------------------------------------------
hooks/post-receive
--
Claws Mail
More information about the Commits
mailing list