[evolution-data-server] [IMAPX] Do not hold queue_lock when calling imapx_server_ref_job()
- From: Milan Crha <mcrha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [evolution-data-server] [IMAPX] Do not hold queue_lock when calling imapx_server_ref_job()
- Date: Tue, 6 May 2014 10:43:21 +0000 (UTC)
commit ea4ef4f07fa18adfe981f4af1be9933904db65ed
Author: Milan Crha <mcrha redhat com>
Date: Tue May 6 12:40:04 2014 +0200
[IMAPX] Do not hold queue_lock when calling imapx_server_ref_job()
Holding the queue_lock when calling imapx_server_ref_job() can cause
deadlock when the store has opened more than one connection and each
is searching for a duplicate job, both/all servers holding respective
queue lock prevents in a search of active jobs in them, both waiting
for a release of the queue_lock of the other server.
camel/providers/imapx/camel-imapx-server.c | 25 ++++++++++---------------
1 files changed, 10 insertions(+), 15 deletions(-)
---
diff --git a/camel/providers/imapx/camel-imapx-server.c b/camel/providers/imapx/camel-imapx-server.c
index 1887360..2b7b8db 100644
--- a/camel/providers/imapx/camel-imapx-server.c
+++ b/camel/providers/imapx/camel-imapx-server.c
@@ -1798,6 +1798,8 @@ imapx_match_active_job (CamelIMAPXServer *is,
return match;
}
+/* Do *not* call this when the queue_lock is held, it can cause
+ deadlock when searching between multiple servers */
static CamelIMAPXJob *
imapx_server_ref_job (CamelIMAPXServer *imapx_server,
CamelIMAPXMailbox *mailbox,
@@ -8132,8 +8134,6 @@ imapx_server_get_message (CamelIMAPXServer *is,
GetMessageData *data;
gboolean registered;
- QUEUE_LOCK (is);
-
job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_GET_MESSAGE, message_uid);
if (job != NULL) {
@@ -8142,8 +8142,6 @@ imapx_server_get_message (CamelIMAPXServer *is,
if (pri > job->pri)
job->pri = pri;
- QUEUE_UNLOCK (is);
-
/* Wait for the job to finish. */
camel_imapx_job_wait (job, NULL);
camel_imapx_job_unref (job);
@@ -8158,10 +8156,10 @@ imapx_server_get_message (CamelIMAPXServer *is,
g_object_unref (cache_stream);
return stream;
}
-
- QUEUE_LOCK (is);
}
+ QUEUE_LOCK (is);
+
mi = camel_folder_summary_get (summary, message_uid);
if (mi == NULL) {
g_set_error (
@@ -8505,19 +8503,18 @@ camel_imapx_server_refresh_info (CamelIMAPXServer *is,
g_return_val_if_fail (CAMEL_IS_IMAPX_SERVER (is), FALSE);
g_return_val_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox), FALSE);
- QUEUE_LOCK (is);
-
/* Don't run concurrent refreshes on the same mailbox.
* If a refresh is already in progress, let it finish
* and return no changes for this refresh request. */
job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_REFRESH_INFO, NULL);
if (job != NULL) {
- QUEUE_UNLOCK (is);
camel_imapx_job_unref (job);
return camel_folder_change_info_new ();
}
+ QUEUE_LOCK (is);
+
data = g_slice_new0 (RefreshInfoData);
data->changes = camel_folder_change_info_new ();
@@ -8780,15 +8777,12 @@ imapx_server_sync_changes (CamelIMAPXServer *is,
/* TODO above code should go into changes_start */
- QUEUE_LOCK (is);
-
job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_SYNC_CHANGES, NULL);
if (job != NULL) {
if (pri > job->pri)
job->pri = pri;
- QUEUE_UNLOCK (is);
camel_imapx_job_unref (job);
imapx_sync_free_user (on_user);
@@ -8798,6 +8792,8 @@ imapx_server_sync_changes (CamelIMAPXServer *is,
return TRUE;
}
+ QUEUE_LOCK (is);
+
data = g_slice_new0 (SyncChangesData);
data->folder = g_object_ref (folder);
data->changed_uids = changed_uids; /* takes ownership */
@@ -8865,16 +8861,15 @@ camel_imapx_server_expunge (CamelIMAPXServer *is,
g_return_val_if_fail (CAMEL_IS_IMAPX_MAILBOX (mailbox), FALSE);
/* Do we really care to wait for this one to finish? */
- QUEUE_LOCK (is);
-
job = imapx_server_ref_job (is, mailbox, IMAPX_JOB_EXPUNGE, NULL);
if (job != NULL) {
- QUEUE_UNLOCK (is);
camel_imapx_job_unref (job);
return TRUE;
}
+ QUEUE_LOCK (is);
+
job = camel_imapx_job_new (cancellable);
job->type = IMAPX_JOB_EXPUNGE;
job->start = imapx_job_expunge_start;
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]