From 6a5e6fbbeaa4e32cd2058019ae87132beba79429 Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 1 May 2001 19:16:14 +0000 Subject: Redo command locking. Since command_lock is recursive, we can just get a * providers/imap/camel-imap-command.c (camel_imap_command): Redo command locking. Since command_lock is recursive, we can just get a lock here, and release it either on error, or when the caller frees the response data. (This simplifies a lot of stuff, and fixes some problems with camel_imap_folder_changed being called without the command_lock locked because of the 2001-03-22 change.) (camel_imap_response_free): (camel_imap_response_free_without_processing): (camel_imap_response_extract): (camel_imap_response_extract_continuation): These all take a CamelImapStore now as well, to deal with locking. * providers/imap/camel-imap-private.h: Add CAMEL_IMAP_STORE_ASSERT_LOCKED, which defaults to a noop, but can be made to call e_mutex_assert_locked. * providers/imap/camel-imap-folder.c, camel-imap-search.c, camel-imap-store.c: Simplify using new locking stuff. Add a few CAMEL_IMAP_STORE_ASSERT_LOCKED checks. svn path=/trunk/; revision=9639 --- camel/providers/imap/camel-imap-command.c | 96 +++++++++----- camel/providers/imap/camel-imap-command.h | 18 +-- camel/providers/imap/camel-imap-folder.c | 79 ++++++------ camel/providers/imap/camel-imap-private.h | 6 + camel/providers/imap/camel-imap-search.c | 10 +- camel/providers/imap/camel-imap-store.c | 199 +++++++++++++----------------- 6 files changed, 211 insertions(+), 197 deletions(-) (limited to 'camel/providers/imap') diff --git a/camel/providers/imap/camel-imap-command.c b/camel/providers/imap/camel-imap-command.c index 36f88917a8..3c36dbfb03 100644 --- a/camel/providers/imap/camel-imap-command.c +++ b/camel/providers/imap/camel-imap-command.c @@ -3,10 +3,10 @@ /* * Authors: - * Dan Winship - * Jeffrey Stedfast + * Dan Winship + * Jeffrey Stedfast * - * Copyright 2000 Helix Code, Inc. (www.helixcode.com) + * Copyright 2000, 2001 Ximian, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -37,6 +37,7 @@ #include "camel-imap-utils.h" #include "camel-imap-folder.h" #include "camel-imap-store.h" +#include "camel-imap-private.h" #include static char *imap_read_untagged (CamelImapStore *store, char *line, @@ -70,8 +71,10 @@ static char *imap_command_strdup_vprintf (CamelImapStore *store, * and quoted strings otherwise. (%S does not support strings that * contain newlines.) * - * This function assumes you have an exclusive lock on the command - * channel/stream. + * On success, the store's command_lock will be locked. It will be freed + * when you call camel_imap_response_free. (The lock is recursive, so + * callers can grab and release it themselves if they need to run + * multiple commands atomically.) * * Return value: %NULL if an error occurred (in which case @ex will * be set). Otherwise, a CamelImapResponse describing the server's @@ -84,6 +87,8 @@ camel_imap_command (CamelImapStore *store, CamelFolder *folder, gchar *cmdbuf; va_list ap; + CAMEL_IMAP_STORE_LOCK (store, command_lock); + /* Check for current folder */ if (folder && (!fmt || folder != store->current_folder)) { CamelImapResponse *response; @@ -94,16 +99,27 @@ camel_imap_command (CamelImapStore *store, CamelFolder *folder, } response = camel_imap_command (store, NULL, ex, "SELECT %S", folder->full_name); - if (!response) + if (!response) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return NULL; + } store->current_folder = folder; camel_object_ref (CAMEL_OBJECT (folder)); camel_imap_folder_selected (folder, response, ex); - if (!fmt) + if (!fmt) { + /* This undoes the level of locking we did, + * but not the level of locking associated with + * "response". + */ + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return response; + } - camel_imap_response_free (response); + /* Contrariwise, this undoes "response"s lock, + * but not our own. + */ + camel_imap_response_free (store, response); } /* Send the command */ @@ -115,8 +131,10 @@ camel_imap_command (CamelImapStore *store, CamelFolder *folder, "A%.5d %s\r\n", store->command++, cmdbuf); g_free (cmdbuf); - if (camel_exception_is_set (ex)) + if (camel_exception_is_set (ex)) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return NULL; + } /* Read the response. */ return imap_read_response (store, ex); @@ -133,15 +151,18 @@ camel_imap_command (CamelImapStore *store, CamelFolder *folder, * * This function assumes you have an exclusive lock on the remote stream. * - * Return value: as for camel_imap_command() + * Return value: as for camel_imap_command(). On failure, the store's + * command_lock will be released. **/ CamelImapResponse * camel_imap_command_continuation (CamelImapStore *store, CamelException *ex, const char *cmdbuf) { if (camel_remote_store_send_string (CAMEL_REMOTE_STORE (store), ex, - "%s\r\n", cmdbuf) < 0) + "%s\r\n", cmdbuf) < 0) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return NULL; + } return imap_read_response (store, ex); } @@ -155,8 +176,10 @@ imap_read_response (CamelImapStore *store, CamelException *ex) /* Read first line */ if (camel_remote_store_recv_line (CAMEL_REMOTE_STORE (store), - &respbuf, ex) < 0) + &respbuf, ex) < 0) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return NULL; + } response = g_new0 (CamelImapResponse, 1); response->folder = store->current_folder; @@ -186,7 +209,7 @@ imap_read_response (CamelImapStore *store, CamelException *ex) } if (!respbuf || camel_exception_is_set (ex)) { - camel_imap_response_free (response); + camel_imap_response_free (store, response); return NULL; } @@ -208,7 +231,7 @@ imap_read_response (CamelImapStore *store, CamelException *ex) camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE, _("Unexpected response from IMAP " "server: %s"), respbuf); - camel_imap_response_free (response); + camel_imap_response_free (store, response); return NULL; } @@ -216,7 +239,7 @@ imap_read_response (CamelImapStore *store, CamelException *ex) camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE, _("IMAP command failed: %s"), retcode ? retcode : _("Unknown error")); - camel_imap_response_free (response); + camel_imap_response_free (store, response); return NULL; } @@ -347,13 +370,14 @@ imap_read_untagged (CamelImapStore *store, char *line, CamelException *ex) /** * camel_imap_response_free: - * @response: a CamelImapResponse: + * @store: the CamelImapStore the response is from + * @response: a CamelImapResponse * * Frees all of the data in @response and processes any untagged - * EXPUNGE and EXISTS responses in it. + * EXPUNGE and EXISTS responses in it. Releases @store's command_lock. **/ void -camel_imap_response_free (CamelImapResponse *response) +camel_imap_response_free (CamelImapStore *store, CamelImapResponse *response) { int i, number, exists = 0; GArray *expunged = NULL; @@ -397,40 +421,47 @@ camel_imap_response_free (CamelImapResponse *response) } g_free (response); + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); } /** - * camel_imap_response_free: + * camel_imap_response_free_without_processing: + * @store: the CamelImapStore the response is from. * @response: a CamelImapResponse: * * Frees all of the data in @response without processing any untagged - * responses. + * responses. Releases @store's command lock. **/ void -camel_imap_response_free_without_processing (CamelImapResponse *response) +camel_imap_response_free_without_processing (CamelImapStore *store, + CamelImapResponse *response) { if (response->folder) { camel_object_unref (CAMEL_OBJECT (response->folder)); response->folder = NULL; } - camel_imap_response_free (response); + camel_imap_response_free (store, response); } /** * camel_imap_response_extract: + * @store: the store the response came from * @response: the response data returned from camel_imap_command * @type: the response type to extract * @ex: a CamelException * * This checks that @response contains a single untagged response of * type @type and returns just that response data. If @response - * doesn't contain the right information, the function will set @ex and - * return %NULL. Either way, @response will be freed. + * doesn't contain the right information, the function will set @ex + * and return %NULL. Either way, @response will be freed and the + * store's command_lock released. * * Return value: the desired response string, which the caller must free. **/ char * -camel_imap_response_extract (CamelImapResponse *response, const char *type, +camel_imap_response_extract (CamelImapStore *store, + CamelImapResponse *response, + const char *type, CamelException *ex) { int len = strlen (type), i; @@ -457,24 +488,26 @@ camel_imap_response_extract (CamelImapResponse *response, const char *type, "%s information"), type); } - camel_imap_response_free (response); + camel_imap_response_free (store, response); return resp; } /** * camel_imap_response_extract_continuation: + * @store: the store the response came from * @response: the response data returned from camel_imap_command * @ex: a CamelException * * This checks that @response contains a continuation response, and * returns just that data. If @response doesn't contain a continuation - * response, the function will set @ex and return %NULL. Either way, - * @response will be freed. + * response, the function will set @ex, release @store's command_lock, + * and return %NULL. Either way, @response will be freed. * * Return value: the desired response string, which the caller must free. **/ char * -camel_imap_response_extract_continuation (CamelImapResponse *response, +camel_imap_response_extract_continuation (CamelImapStore *store, + CamelImapResponse *response, CamelException *ex) { char *status; @@ -482,14 +515,15 @@ camel_imap_response_extract_continuation (CamelImapResponse *response, if (response->status && !strncmp (response->status, "+ ", 2)) { status = response->status; response->status = NULL; - camel_imap_response_free (response); + CAMEL_IMAP_STORE_LOCK (store, command_lock); + camel_imap_response_free (store, response); return status; } camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE, _("Unexpected OK response from IMAP server: %s"), response->status); - camel_imap_response_free (response); + camel_imap_response_free (store, response); return NULL; } diff --git a/camel/providers/imap/camel-imap-command.h b/camel/providers/imap/camel-imap-command.h index 5b841525fa..ba9ca7010a 100644 --- a/camel/providers/imap/camel-imap-command.h +++ b/camel/providers/imap/camel-imap-command.h @@ -3,10 +3,10 @@ /* * Authors: - * Dan Winship - * Jeffrey Stedfast + * Dan Winship + * Jeffrey Stedfast * - * Copyright (C) 2000 Helix Code, Inc. + * Copyright (C) 2000, 2001 Ximian, Inc. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License as @@ -50,12 +50,16 @@ CamelImapResponse *camel_imap_command_continuation (CamelImapStore *store, CamelException *ex, const char *cmdbuf); -void camel_imap_response_free (CamelImapResponse *response); -void camel_imap_response_free_without_processing(CamelImapResponse *response); -char *camel_imap_response_extract (CamelImapResponse *response, +void camel_imap_response_free (CamelImapStore *store, + CamelImapResponse *response); +void camel_imap_response_free_without_processing(CamelImapStore *store, + CamelImapResponse *response); +char *camel_imap_response_extract (CamelImapStore *store, + CamelImapResponse *response, const char *type, CamelException *ex); -char *camel_imap_response_extract_continuation (CamelImapResponse *response, +char *camel_imap_response_extract_continuation (CamelImapStore *store, + CamelImapResponse *response, CamelException *ex); #endif /* CAMEL_IMAP_COMMAND_H */ diff --git a/camel/providers/imap/camel-imap-folder.c b/camel/providers/imap/camel-imap-folder.c index 17d75b1abb..4ee65371fd 100644 --- a/camel/providers/imap/camel-imap-folder.c +++ b/camel/providers/imap/camel-imap-folder.c @@ -179,15 +179,13 @@ camel_imap_folder_new (CamelStore *parent, const char *folder_name, } if (camel_imap_store_check_online (imap_store, NULL)) { - CAMEL_IMAP_STORE_LOCK (imap_store, command_lock); response = camel_imap_command (imap_store, folder, ex, NULL); - CAMEL_IMAP_STORE_UNLOCK (imap_store, command_lock); - - if (!response) { + if (response) + camel_imap_response_free (imap_store, response); + else { camel_object_unref (CAMEL_OBJECT (folder)); return NULL; } - camel_imap_response_free (response); } return folder; @@ -206,6 +204,8 @@ camel_imap_folder_selected (CamelFolder *folder, CamelImapResponse *response, int i, count; char *resp; + CAMEL_IMAP_STORE_ASSERT_LOCKED (folder->parent_store, command_lock); + count = camel_folder_summary_count (folder->summary); for (i = 0; i < response->untagged->len; i++) { @@ -250,6 +250,8 @@ camel_imap_folder_selected (CamelFolder *folder, CamelImapResponse *response, } if (count != 0) { + CamelImapStore *store = CAMEL_IMAP_STORE (folder->parent_store); + /* Similarly, if the UID of the highest message we * know about has changed, then that indicates that * messages have been both added and removed, so we @@ -258,8 +260,7 @@ camel_imap_folder_selected (CamelFolder *folder, CamelImapResponse *response, * is selected, and we don't want camel_imap_command * to worry about it.) */ - response = camel_imap_command (CAMEL_IMAP_STORE (folder->parent_store), - NULL, ex, "FETCH %d UID", count); + response = camel_imap_command (store, NULL, ex, "FETCH %d UID", count); if (!response) return; uid = 0; @@ -280,7 +281,7 @@ camel_imap_folder_selected (CamelFolder *folder, CamelImapResponse *response, uid = strtoul (g_datalist_get_data (&fetch_data, "UID"), NULL, 10); g_datalist_clear (&fetch_data); } - camel_imap_response_free_without_processing (response); + camel_imap_response_free_without_processing (store, response); info = camel_folder_summary_index (folder->summary, count - 1); val = strtoul (camel_message_info_uid (info), NULL, 10); @@ -346,6 +347,8 @@ imap_rescan (CamelFolder *folder, int exists, CamelException *ex) GArray *removed; GData *fetch_data; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + camel_operation_start(NULL, _("Scanning IMAP folder")); /* Get UIDs and flags of all messages. */ @@ -370,7 +373,7 @@ imap_rescan (CamelFolder *folder, int exists, CamelException *ex) g_datalist_clear (&fetch_data); g_ptr_array_remove_index_fast (response->untagged, i--); } - camel_imap_response_free_without_processing (response); + camel_imap_response_free_without_processing (store, response); } /* If we find a UID in the summary that doesn't correspond to @@ -510,6 +513,8 @@ imap_sync (CamelFolder *folder, gboolean expunge, CamelException *ex) max = camel_folder_summary_count (folder->summary); + CAMEL_IMAP_STORE_LOCK (store, command_lock); + /* If we're expunging then we don't need to be precise about the * flags of deleted messages. Just add \Deleted to anything that * should have it. @@ -524,16 +529,16 @@ imap_sync (CamelFolder *folder, gboolean expunge, CamelException *ex) g_ptr_array_free (matches, TRUE); camel_folder_summary_touch (folder->summary); - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, folder, ex, "UID STORE %s +FLAGS.SILENT \\Deleted", set); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); g_free (set); if (response) - camel_imap_response_free (response); - if (camel_exception_is_set (ex)) + camel_imap_response_free (store, response); + if (camel_exception_is_set (ex)) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return; + } } /* OK, now, find a message with changed flags, find all of the @@ -554,15 +559,13 @@ imap_sync (CamelFolder *folder, gboolean expunge, CamelException *ex) CAMEL_IMAP_SERVER_FLAGS | CAMEL_MESSAGE_FOLDER_FLAGGED, &set); camel_folder_summary_info_free (folder->summary, info); - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, folder, ex, "UID STORE %s FLAGS.SILENT %s", set, flaglist); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); g_free (set); g_free (flaglist); if (response) - camel_imap_response_free (response); + camel_imap_response_free (store, response); if (!camel_exception_is_set (ex)) { for (j = 0; j < matches->len; j++) { info = matches->pdata[j]; @@ -578,28 +581,27 @@ imap_sync (CamelFolder *folder, gboolean expunge, CamelException *ex) } g_ptr_array_free (matches, TRUE); - if (camel_exception_is_set (ex)) + if (camel_exception_is_set (ex)) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return; + } } if (expunge) { - CAMEL_IMAP_STORE_LOCK(store, command_lock); response = camel_imap_command (store, folder, ex, "EXPUNGE"); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); - camel_imap_response_free (response); + camel_imap_response_free (store, response); } if (!response) { /* We didn't sync or expunge anything... Do a noop so * the server gets a chance to tell us any news it has. */ - CAMEL_IMAP_STORE_LOCK(store, command_lock); response = camel_imap_command (store, folder, ex, "NOOP"); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); - camel_imap_response_free (response); + camel_imap_response_free (store, response); } camel_folder_summary_save (folder->summary); + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); camel_operation_end(NULL); } @@ -669,7 +671,6 @@ imap_append_message (CamelFolder *folder, CamelMimeMessage *message, camel_object_unref (CAMEL_OBJECT (crlf_filter)); camel_object_unref (CAMEL_OBJECT (memstream)); - CAMEL_IMAP_STORE_LOCK(store, command_lock); response = camel_imap_command (store, NULL, ex, "APPEND %S%s%s {%d}", folder->full_name, flagstr ? " " : "", flagstr ? flagstr : "", ba->len); @@ -677,13 +678,11 @@ imap_append_message (CamelFolder *folder, CamelMimeMessage *message, if (!response) { g_byte_array_free (ba, TRUE); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); return; } - result = camel_imap_response_extract_continuation (response, ex); + result = camel_imap_response_extract_continuation (store, response, ex); if (!result) { g_byte_array_free (ba, TRUE); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); return; } g_free (result); @@ -692,7 +691,6 @@ imap_append_message (CamelFolder *folder, CamelMimeMessage *message, g_byte_array_append (ba, "\0", 3); response = camel_imap_command_continuation (store, ex, ba->data); g_byte_array_free (ba, TRUE); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); if (!response) return; @@ -714,7 +712,7 @@ imap_append_message (CamelFolder *folder, CamelMimeMessage *message, } } - camel_imap_response_free (response); + camel_imap_response_free (store, response); } static void @@ -787,23 +785,21 @@ imap_copy_messages_to (CamelFolder *source, GPtrArray *uids, return; /* Now copy the messages */ - CAMEL_IMAP_STORE_LOCK(store, command_lock); set = imap_uid_array_to_set (source->summary, uids); response = camel_imap_command (store, source, ex, "UID COPY %s %S", set, destination->full_name); if (response && (store->capabilities & IMAP_CAPABILITY_UIDPLUS)) handle_copyuid (response, source, destination); - camel_imap_response_free (response); + camel_imap_response_free (store, response); g_free (set); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); if (camel_exception_is_set (ex)) return; /* Force the destination folder to notice its new messages. */ response = camel_imap_command (store, destination, NULL, "NOOP"); - camel_imap_response_free (response); + camel_imap_response_free (store, response); } static void @@ -1025,10 +1021,8 @@ imap_get_message (CamelFolder *folder, const char *uid, CamelException *ex) return NULL; } - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, folder, ex, "UID FETCH %s BODY", uid); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); if (!response) { camel_folder_summary_info_free (folder->summary, mi); return NULL; @@ -1047,7 +1041,7 @@ imap_get_message (CamelFolder *folder, const char *uid, CamelException *ex) if (body) imap_parse_body (&body, folder, mi->content); g_datalist_clear (&fetch_data); - camel_imap_response_free (response); + camel_imap_response_free (store, response); if (!mi->content->type) { camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE, @@ -1079,9 +1073,10 @@ imap_update_summary (CamelFolder *folder, GData *fetch_data; CamelStream *stream; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + first = camel_folder_summary_count (folder->summary) + 1; - /* We already have the command lock */ response = camel_imap_command (store, folder, ex, "FETCH %d:* (UID FLAGS RFC822.SIZE)", first); if (!response) return; @@ -1119,7 +1114,7 @@ imap_update_summary (CamelFolder *folder, store->server_level >= IMAP_LEVEL_IMAP4REV1 ? "HEADER" : "0", FALSE, ex); if (!stream) { - camel_imap_response_free_without_processing (response); + camel_imap_response_free_without_processing (store, response); /* XXX messages */ return; } @@ -1163,7 +1158,7 @@ imap_update_summary (CamelFolder *folder, g_datalist_clear (&fetch_data); } - camel_imap_response_free_without_processing (response); + camel_imap_response_free_without_processing (store, response); for (i = 0; i < messages->len; i++) { mi = messages->pdata[i]; @@ -1191,6 +1186,8 @@ camel_imap_folder_changed (CamelFolder *folder, int exists, CamelMessageInfo *info; int len; + CAMEL_IMAP_STORE_ASSERT_LOCKED (folder->parent_store, command_lock); + changes = camel_folder_change_info_new (); if (expunged) { int i, id; @@ -1244,7 +1241,6 @@ camel_imap_folder_fetch_data (CamelImapFolder *imap_folder, const char *uid, return NULL; } - CAMEL_IMAP_STORE_LOCK (store, command_lock); if (store->server_level < IMAP_LEVEL_IMAP4REV1 && !*section_text) { response = camel_imap_command (store, folder, ex, "UID FETCH %s RFC822.PEEK", @@ -1254,7 +1250,6 @@ camel_imap_folder_fetch_data (CamelImapFolder *imap_folder, const char *uid, "UID FETCH %s BODY.PEEK[%s]", uid, section_text); } - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); if (!response) { CAMEL_IMAP_FOLDER_UNLOCK (imap_folder, cache_lock); return NULL; @@ -1270,7 +1265,7 @@ camel_imap_folder_fetch_data (CamelImapFolder *imap_folder, const char *uid, g_datalist_clear (&fetch_data); stream = NULL; } - camel_imap_response_free (response); + camel_imap_response_free (store, response); CAMEL_IMAP_FOLDER_UNLOCK (imap_folder, cache_lock); if (!stream) { camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_UNAVAILABLE, diff --git a/camel/providers/imap/camel-imap-private.h b/camel/providers/imap/camel-imap-private.h index 811efc5576..6004840f88 100644 --- a/camel/providers/imap/camel-imap-private.h +++ b/camel/providers/imap/camel-imap-private.h @@ -47,9 +47,15 @@ struct _CamelImapStorePrivate { #ifdef ENABLE_THREADS #define CAMEL_IMAP_STORE_LOCK(f, l) (e_mutex_lock(((CamelImapStore *)f)->priv->l)) #define CAMEL_IMAP_STORE_UNLOCK(f, l) (e_mutex_unlock(((CamelImapStore *)f)->priv->l)) +# if 0 +# define CAMEL_IMAP_STORE_ASSERT_LOCKED(f, l) (e_mutex_assert_locked(((CamelImapStore *)f)->priv->l)) +# else +# define CAMEL_IMAP_STORE_ASSERT_LOCKED(f, l) +# endif #else #define CAMEL_IMAP_STORE_LOCK(f, l) #define CAMEL_IMAP_STORE_UNLOCK(f, l) +#define CAMEL_IMAP_STORE_ASSERT_LOCKED(f, l) #endif struct _CamelImapFolderPrivate { diff --git a/camel/providers/imap/camel-imap-search.c b/camel/providers/imap/camel-imap-search.c index 4b913b10c2..be8ed85eca 100644 --- a/camel/providers/imap/camel-imap-search.c +++ b/camel/providers/imap/camel-imap-search.c @@ -3,9 +3,9 @@ /* * Authors: - * Dan Winship + * Dan Winship * - * Copyright 2000 Helix Code, Inc. (www.helixcode.com) + * Copyright 2000, 2001 Ximian, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -80,8 +80,6 @@ imap_body_contains (struct _ESExp *f, int argc, struct _ESExpResult **argv, CamelMessageInfo *info; GHashTable *uid_hash = NULL; - CAMEL_IMAP_STORE_LOCK(store, command_lock); - if (s->current) { uid = camel_message_info_uid (s->current); r = e_sexp_result_new(f, ESEXP_RES_BOOL); @@ -97,11 +95,9 @@ imap_body_contains (struct _ESExp *f, int argc, struct _ESExpResult **argv, value); } - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); - if (!response) return r; - result = camel_imap_response_extract (response, "SEARCH", NULL); + result = camel_imap_response_extract (store, response, "SEARCH", NULL); if (!result) return r; diff --git a/camel/providers/imap/camel-imap-store.c b/camel/providers/imap/camel-imap-store.c index 9921de90c5..8511b3e9f7 100644 --- a/camel/providers/imap/camel-imap-store.c +++ b/camel/providers/imap/camel-imap-store.c @@ -2,9 +2,11 @@ /* camel-imap-store.c : class for an imap store */ /* - * Authors: Jeffrey Stedfast + * Authors: + * Dan Winship + * Jeffrey Stedfast * - * Copyright 2000 Helix Code, Inc. (www.helixcode.com) + * Copyright 2000, 2001 Ximian, Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -251,6 +253,8 @@ connect_to_server (CamelService *service, CamelException *ex) char *result, *buf, *capa, *lasts; int i; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + if (!CAMEL_SERVICE_CLASS (remote_store_class)->connect (service, ex)) return FALSE; @@ -270,7 +274,7 @@ connect_to_server (CamelService *service, CamelException *ex) response = camel_imap_command (store, NULL, ex, "CAPABILITY"); if (!response) return FALSE; - result = camel_imap_response_extract (response, "CAPABILITY ", ex); + result = camel_imap_response_extract (store, response, "CAPABILITY ", ex); if (!result) return FALSE; @@ -312,8 +316,12 @@ query_auth_types (CamelService *service, CamelException *ex) CamelImapStore *store = CAMEL_IMAP_STORE (service); CamelServiceAuthType *authtype; GList *types, *sasl_types, *t, *next; + gboolean connected; - if (!connect_to_server (service, ex)) + CAMEL_IMAP_STORE_LOCK (store, command_lock); + connected = connect_to_server (service, ex); + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); + if (!connected) return NULL; types = CAMEL_SERVICE_CLASS (remote_store_class)->query_auth_types (service, ex); @@ -368,11 +376,12 @@ try_auth (CamelImapStore *store, const char *mech, CamelException *ex) char *resp; char *sasl_resp; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + sasl = camel_sasl_new ("imap", mech, CAMEL_SERVICE (store)); sasl_resp = camel_sasl_challenge_base64 (sasl, NULL, ex); - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, NULL, ex, "AUTHENTICATE %s%s%s", mech, sasl_resp ? " " : "", sasl_resp ? sasl_resp : ""); @@ -380,7 +389,7 @@ try_auth (CamelImapStore *store, const char *mech, CamelException *ex) goto lose; while (!camel_sasl_authenticated (sasl)) { - resp = camel_imap_response_extract_continuation (response, ex); + resp = camel_imap_response_extract_continuation (store, response, ex); if (!resp) goto lose; @@ -395,7 +404,7 @@ try_auth (CamelImapStore *store, const char *mech, CamelException *ex) goto lose; } - resp = camel_imap_response_extract_continuation (response, NULL); + resp = camel_imap_response_extract_continuation (store, response, NULL); if (resp) { /* Oops. SASL claims we're done, but the IMAP server * doesn't think so... @@ -406,14 +415,13 @@ try_auth (CamelImapStore *store, const char *mech, CamelException *ex) camel_object_unref (CAMEL_OBJECT (sasl)); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); return TRUE; break_and_lose: /* Get the server out of "waiting for continuation data" mode. */ response = camel_imap_command_continuation (store, NULL, "*"); if (response) - camel_imap_response_free (response); + camel_imap_response_free (store, response); lose: if (!camel_exception_is_set (ex)) { @@ -423,8 +431,6 @@ try_auth (CamelImapStore *store, const char *mech, CamelException *ex) camel_object_unref (CAMEL_OBJECT (sasl)); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); - return FALSE; } @@ -438,6 +444,8 @@ imap_auth_loop (CamelService *service, CamelException *ex) char *errbuf = NULL; gboolean authenticated = FALSE; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + if (service->url->authmech) { if (!g_hash_table_lookup (store->authtypes, service->url->authmech)) { camel_exception_setv (ex, CAMEL_EXCEPTION_SERVICE_CANT_AUTHENTICATE, @@ -506,14 +514,12 @@ imap_auth_loop (CamelService *service, CamelException *ex) if (authtype) authenticated = try_auth (store, authtype->authproto, ex); else { - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, NULL, ex, "LOGIN %S %S", service->url->user, service->url->passwd); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); if (response) { - camel_imap_response_free (response); + camel_imap_response_free (store, response); authenticated = TRUE; } } @@ -534,16 +540,15 @@ imap_connect (CamelService *service, CamelException *ex) CamelImapStore *store = CAMEL_IMAP_STORE (service); if (camel_imap_store_check_online (store, NULL)) { - if (!connect_to_server (service, ex)) - return FALSE; - if (!imap_auth_loop (service, ex)) { - camel_service_disconnect (service, TRUE, NULL); - return FALSE; - } - if (!imap_store_setup_online (store, ex)) { + CAMEL_IMAP_STORE_LOCK (store, command_lock); + if (!connect_to_server (service, ex) || + !imap_auth_loop (service, ex) || + !imap_store_setup_online (store, ex)) { + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); camel_service_disconnect (service, TRUE, NULL); return FALSE; } + CAMEL_IMAP_STORE_UNLOCK (store, command_lock); } else imap_store_setup_offline (store, ex); @@ -561,6 +566,8 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) char *result, *name, *path; FILE *storeinfo; + CAMEL_IMAP_STORE_ASSERT_LOCKED (store, command_lock); + path = g_strdup_printf ("%s/storeinfo", store->storage_path); storeinfo = fopen (path, "w"); if (!storeinfo) @@ -574,13 +581,11 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) /* Get namespace and hierarchy separator */ if ((store->capabilities & IMAP_CAPABILITY_NAMESPACE) && !(store->parameters & IMAP_PARAM_OVERRIDE_NAMESPACE)) { - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, NULL, ex, "NAMESPACE"); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); if (!response) return FALSE; - result = camel_imap_response_extract (response, "NAMESPACE", ex); + result = camel_imap_response_extract (store, response, "NAMESPACE", ex); if (!result) return FALSE; @@ -604,7 +609,6 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) store->namespace = g_strdup (""); if (!store->dir_sep) { - CAMEL_IMAP_STORE_LOCK (store, command_lock); if (store->server_level >= IMAP_LEVEL_IMAP4REV1) { /* This idiom means "tell me the hierarchy separator * for the given path, even if that path doesn't exist. @@ -621,12 +625,11 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) "LIST \"\" %S", store->namespace); } - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); if (!response) return FALSE; - result = camel_imap_response_extract (response, "LIST", NULL); + result = camel_imap_response_extract (store, response, "LIST", NULL); if (result) { imap_parse_list_response (result, NULL, &store->dir_sep, NULL); g_free (result); @@ -641,9 +644,7 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) if (CAMEL_STORE (store)->flags & CAMEL_STORE_SUBSCRIPTIONS) { /* Get subscribed folders */ - CAMEL_IMAP_STORE_LOCK (store, command_lock); response = camel_imap_command (store, NULL, ex, "LSUB \"\" \"*\""); - CAMEL_IMAP_STORE_UNLOCK (store, command_lock); if (!response) return FALSE; store->subscribed_folders = g_hash_table_new (g_str_hash, g_str_equal); @@ -661,7 +662,7 @@ imap_store_setup_online (CamelImapStore *store, CamelException *ex) GINT_TO_POINTER (1)); camel_file_util_encode_string (storeinfo, result); } - camel_imap_response_free (response); + camel_imap_response_free (store, response); } fclose (storeinfo); @@ -716,13 +717,8 @@ imap_disconnect (CamelService *service, gboolean clean, CamelException *ex) CamelImapResponse *response; if (store->connected && clean) { - /* send the logout command */ - - /* NB: this lock probably isn't required */ - CAMEL_IMAP_STORE_LOCK(store, command_lock); response = camel_imap_command (store, NULL, ex, "LOGOUT"); - CAMEL_IMAP_STORE_UNLOCK(store, command_lock); - camel_imap_response_free (response); + camel_imap_response_free (store, response); } store->connected = FALSE; if (store->current_folder) { @@ -752,7 +748,6 @@ imap_disconnect (CamelService *service, gboolean clean, CamelException *ex) return CAMEL_SERVICE_CLASS (remote_store_class)->disconnect (service, clean, ex); } -/* NOTE: Must have imap_store::command_lock before calling this */ static gboolean imap_folder_exists (CamelImapStore *store, const char *folder_name, gboolean *selectable, char **short_name, @@ -774,7 +769,7 @@ imap_folder_exists (CamelImapStore *store, const char *folder_name, folder_name); if (!response) return FALSE; - result = camel_imap_response_extract (response, "LIST", ex); + result = camel_imap_response_extract (store, response, "LIST", ex); if (!result) return FALSE; @@ -794,7 +789,6 @@ imap_folder_exists (CamelImapStore *store, const char *folder_name, return TRUE; } -/* NOTE: Must have imap_store::command_lock before calling this */ static gboolean imap_create (CamelImapStore *store, const char *folder_name, CamelException *ex) @@ -803,7 +797,7 @@ imap_create (CamelImapStore *store, const char *folder_name, response = camel_imap_command (store, NULL, ex, "CREATE %S", folder_name); - camel_imap_response_free (response); + camel_imap_response_free (store, response); return !camel_exception_is_set (ex); } @@ -853,7 +847,6 @@ get_folder (CamelStore *store, const char *folder_name, guint32 flags, CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); return NULL; } - CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); } else { selectable = g_hash_table_lookup (imap_store->subscribed_folders, folder_name) != NULL; short_name = strrchr (folder_name, imap_store->dir_sep); @@ -861,6 +854,9 @@ get_folder (CamelStore *store, const char *folder_name, guint32 flags, short_name = g_strdup (short_name + 1); else short_name = g_strdup (folder_name); + + /* Need to lock here for parallelism with the online case */ + CAMEL_IMAP_STORE_LOCK(imap_store, command_lock); } if (!selectable) { @@ -868,6 +864,7 @@ get_folder (CamelStore *store, const char *folder_name, guint32 flags, _("%s is not a selectable folder"), folder_name); g_free (short_name); + CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); return NULL; } @@ -882,6 +879,7 @@ get_folder (CamelStore *store, const char *folder_name, guint32 flags, _("Could not create directory %s: %s"), folder_dir, g_strerror (errno)); } + CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); g_free (folder_dir); g_free (short_name); @@ -990,17 +988,14 @@ get_subscribed_folders_by_hand (CamelImapStore *imap_store, const char *top, copy_folder_name, names); for (i = 0; i < names->len; i++) { - CAMEL_IMAP_STORE_LOCK (imap_store, command_lock); response = camel_imap_command (imap_store, NULL, ex, "LIST \"\" %S", names->pdata[i]); - CAMEL_IMAP_STORE_UNLOCK (imap_store, command_lock); - if (!response) { g_ptr_array_free (names, TRUE); return; } - result = camel_imap_response_extract (response, "LIST", NULL); + result = camel_imap_response_extract (imap_store, response, "LIST", NULL); if (!result) { g_hash_table_remove (imap_store->subscribed_folders, names->pdata[i]); @@ -1035,11 +1030,9 @@ get_folders_online (CamelImapStore *imap_store, const char *pattern, if (!camel_remote_store_connected (CAMEL_REMOTE_STORE (imap_store), ex)) return; - CAMEL_IMAP_STORE_LOCK (imap_store, command_lock); response = camel_imap_command (imap_store, NULL, ex, "%s \"\" %S", lsub ? "LSUB" : "LIST", pattern); - CAMEL_IMAP_STORE_UNLOCK (imap_store, command_lock); if (!response) return; @@ -1049,7 +1042,7 @@ get_folders_online (CamelImapStore *imap_store, const char *pattern, if (fi) g_ptr_array_add (folders, fi); } - camel_imap_response_free (response); + camel_imap_response_free (imap_store, response); } static void @@ -1058,13 +1051,11 @@ get_unread_online (CamelImapStore *imap_store, CamelFolderInfo *fi) CamelImapResponse *response; char *status, *p; - CAMEL_IMAP_STORE_LOCK (imap_store, command_lock); response = camel_imap_command (imap_store, NULL, NULL, "STATUS %S (UNSEEN)", fi->full_name); - CAMEL_IMAP_STORE_UNLOCK (imap_store, command_lock); if (!response) return; - status = camel_imap_response_extract (response, "STATUS", NULL); + status = camel_imap_response_extract (imap_store, response, "STATUS", NULL); if (!status) return; @@ -1283,40 +1274,35 @@ subscribe_folder (CamelStore *store, const char *folder_name, { CamelImapStore *imap_store = CAMEL_IMAP_STORE (store); CamelImapResponse *response; - + CamelFolderInfo *fi; + char *name; + if (!camel_imap_store_check_online (imap_store, ex)) return; if (!camel_remote_store_connected (CAMEL_REMOTE_STORE (store), ex)) return; - CAMEL_IMAP_STORE_LOCK(imap_store, command_lock); response = camel_imap_command (imap_store, NULL, ex, "SUBSCRIBE %S", folder_name); - CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); - if (response) { - CamelFolderInfo *fi; - char *name; - - g_hash_table_insert (imap_store->subscribed_folders, - g_strdup (folder_name), - GUINT_TO_POINTER (1)); - - name = strrchr (folder_name, imap_store->dir_sep); - if (name) - name++; - - fi = g_new0 (CamelFolderInfo, 1); - fi->full_name = g_strdup (folder_name); - fi->name = g_strdup (name); - fi->url = g_strdup_printf ("%s/%s", imap_store->base_url, folder_name); - fi->unread_message_count = -1; - - camel_object_trigger_event (CAMEL_OBJECT (store), - "folder_created", fi); - - camel_folder_info_free (fi); - } - camel_imap_response_free (response); + if (!response) + return; + camel_imap_response_free (imap_store, response); + + g_hash_table_insert (imap_store->subscribed_folders, + g_strdup (folder_name), GUINT_TO_POINTER (1)); + + name = strrchr (folder_name, imap_store->dir_sep); + if (name) + name++; + + fi = g_new0 (CamelFolderInfo, 1); + fi->full_name = g_strdup (folder_name); + fi->name = g_strdup (name); + fi->url = g_strdup_printf ("%s/%s", imap_store->base_url, folder_name); + fi->unread_message_count = -1; + + camel_object_trigger_event (CAMEL_OBJECT (store), "folder_created", fi); + camel_folder_info_free (fi); } static void @@ -1326,42 +1312,37 @@ unsubscribe_folder (CamelStore *store, const char *folder_name, CamelImapStore *imap_store = CAMEL_IMAP_STORE (store); CamelImapResponse *response; gpointer key, value; - + CamelFolderInfo *fi; + char *name; + if (!camel_imap_store_check_online (imap_store, ex)) return; if (!camel_remote_store_connected (CAMEL_REMOTE_STORE (store), ex)) return; - CAMEL_IMAP_STORE_LOCK(imap_store, command_lock); response = camel_imap_command (imap_store, NULL, ex, "UNSUBSCRIBE %S", folder_name); - CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); - if (response) { - CamelFolderInfo *fi; - char *name; - - g_hash_table_lookup_extended (imap_store->subscribed_folders, - folder_name, &key, &value); - g_hash_table_remove (imap_store->subscribed_folders, - folder_name); - g_free (key); - - name = strrchr (folder_name, imap_store->dir_sep); - if (name) - name++; - - fi = g_new0 (CamelFolderInfo, 1); - fi->full_name = g_strdup (folder_name); - fi->name = g_strdup (name); - fi->url = g_strdup_printf ("%s/%s", imap_store->base_url, folder_name); - fi->unread_message_count = -1; - - camel_object_trigger_event (CAMEL_OBJECT (store), - "folder_deleted", fi); - - camel_folder_info_free (fi); - } - camel_imap_response_free (response); + if (!response) + return; + camel_imap_response_free (imap_store, response); + + g_hash_table_lookup_extended (imap_store->subscribed_folders, + folder_name, &key, &value); + g_hash_table_remove (imap_store->subscribed_folders, folder_name); + g_free (key); + + name = strrchr (folder_name, imap_store->dir_sep); + if (name) + name++; + + fi = g_new0 (CamelFolderInfo, 1); + fi->full_name = g_strdup (folder_name); + fi->name = g_strdup (name); + fi->url = g_strdup_printf ("%s/%s", imap_store->base_url, folder_name); + fi->unread_message_count = -1; + + camel_object_trigger_event (CAMEL_OBJECT (store), "folder_deleted", fi); + camel_folder_info_free (fi); } static void @@ -1370,10 +1351,8 @@ imap_keepalive (CamelRemoteStore *store) CamelImapStore *imap_store = CAMEL_IMAP_STORE (store); CamelImapResponse *response; - CAMEL_IMAP_STORE_LOCK(imap_store, command_lock); response = camel_imap_command (imap_store, NULL, NULL, "NOOP"); - CAMEL_IMAP_STORE_UNLOCK(imap_store, command_lock); - camel_imap_response_free (response); + camel_imap_response_free (imap_store, response); } gboolean -- cgit