balsa r8020 - in trunk: . libbalsa libbalsa/imap po src

[pawels svn gnome org]

Author: pawels
Date: Sun Dec 21 21:27:44 2008
New Revision: 8020
URL: http://svn.gnome.org/viewvc/balsa?rev=8020&view=rev

Log:
* configure.in: we require gnome-keyring >= 1.0
Remove some of the race conditions as found with helgrind.
* src/balsa-app.c: do not manually destroy widgets.
* src/main.c: cleanup explicitly before exit from main, no signals needed.
* libbalsa/mailbox_{imap,local}.c: lock mailbox.
* libbalsa/mailbox.c: keep the lock a bit longer.
* libbalsa/libbalsa.c: preregister more object types.
* libbalsa/imap/imap-{commands,handle}.c: lock the handle.


Modified:
   trunk/ChangeLog
   trunk/configure.in
   trunk/libbalsa/imap/imap-commands.c
   trunk/libbalsa/imap/imap-handle.c
   trunk/libbalsa/libbalsa.c
   trunk/libbalsa/mailbox.c
   trunk/libbalsa/mailbox_imap.c
   trunk/libbalsa/mailbox_local.c
   trunk/po/et.po
   trunk/src/balsa-app.c
   trunk/src/main.c

Modified: trunk/configure.in
==============================================================================
--- trunk/configure.in	(original)
+++ trunk/configure.in	Sun Dec 21 21:27:44 2008
@@ -289,7 +289,7 @@
    gnome-vfs-2.0 gnome-vfs-module-2.0 libbonobo-2.0"
    gnome_print_extras="libgnomeprint-2.2 >= 2.1.4 libgnomeprintui-2.2 >= 2.1.4"
    AC_MSG_CHECKING([whether we have gnome-keyring])
-   if $PKG_CONFIG --exists gnome-keyring-1; then
+   if $PKG_CONFIG --atleast-version=1.0 gnome-keyring-1; then
       gnome_extras="$gnome_extras gnome-keyring-1"
       AC_DEFINE(HAVE_GNOME_KEYRING,1,[Defined when gnome-keyring is there.])
       # Work around http://bugzilla.gnome.org/show_bug.cgi?id=556530

Modified: trunk/libbalsa/imap/imap-commands.c
==============================================================================
--- trunk/libbalsa/imap/imap-commands.c	(original)
+++ trunk/libbalsa/imap/imap-commands.c	Sun Dec 21 21:27:44 2008
@@ -668,9 +668,11 @@
   ImapResponse rc;
   IMAP_REQUIRED_STATE1(h, IMHS_SELECTED, IMR_BAD);
 
+  HANDLE_LOCK(h);
   rc = imap_cmd_exec(h, "CLOSE");  
   if(rc == IMR_OK)
     h->state = IMHS_AUTHENTICATED;
+  HANDLE_UNLOCK(h);
   return rc;
 }
 

Modified: trunk/libbalsa/imap/imap-handle.c
==============================================================================
--- trunk/libbalsa/imap/imap-handle.c	(original)
+++ trunk/libbalsa/imap/imap-handle.c	Sun Dec 21 21:27:44 2008
@@ -348,15 +348,21 @@
   ImapResponse rc = IMR_UNTAGGED;
   int retval;
   unsigned async_cmd;
+  gboolean retval_async;
+
   g_return_val_if_fail(h, FALSE);
 
+  if(HANDLE_TRYLOCK(h) != 0) 
+    return FALSE;/* async data on already locked handle? Don't try again. */
   if(ASYNC_DEBUG) printf("async_process() ENTER\n");
   if(h->state == IMHS_DISCONNECTED) {
     if(ASYNC_DEBUG) printf("async_process() on disconnected\n");
+    HANDLE_UNLOCK(h);
     return FALSE;
   }
   if( (condition & G_IO_HUP) == G_IO_HUP) {
       imap_handle_disconnect(h);
+      HANDLE_UNLOCK(h);
       return FALSE;
   }
 
@@ -371,6 +377,7 @@
              "Last message was: \"%s\" - shutting down connection.\n",
              rc, h->last_msg);
       imap_handle_disconnect(h);
+      HANDLE_UNLOCK(h);
       return FALSE;
     }
     async_cmd = cmdi_get_pending(h->cmd_info);
@@ -391,7 +398,9 @@
     printf("async_process() sio: %d rc: %d returns %d (%d cmds in queue)\n",
            retval, rc, !h->idle_issued && async_cmd == 0,
            g_list_length(h->cmd_info));
-  return h->idle_issued || async_cmd != 0;
+  retval_async = h->idle_issued || async_cmd != 0;
+  HANDLE_UNLOCK(h);
+  return retval_async;
 }
 
 static gboolean
@@ -661,6 +670,9 @@
   ImapMboxHandle *h = (ImapMboxHandle*)arg;
   int ok = 1;
 
+  /* No reason to lock the handle here: if we get here, we have
+     already been performing some operation and keep the handle
+     locked. */
   if(h->user_cb) {
     h->user_cb(IME_TIMEOUT, h->user_arg, &ok);
     if(ok) {
@@ -678,6 +690,7 @@
   static const int SIO_BUFSZ=8192;
   ImapResponse resp;
   const char *service = "imap";
+  HANDLE_LOCK(handle);
 
   /* reset some handle status */
   handle->op_cancelled = FALSE;
@@ -695,12 +708,16 @@
 #endif
 
   handle->sd = imap_socket_open(handle->host, service);
-  if(handle->sd<0) return IMAP_CONNECT_FAILED;
+  if(handle->sd<0) {
+    HANDLE_UNLOCK(handle);
+    return IMAP_CONNECT_FAILED;
+  }
   
   /* Add buffering to the socket */
   handle->sio = sio_attach(handle->sd, handle->sd, SIO_BUFSZ);
   if (handle->sio == NULL) {
     close(handle->sd);
+    HANDLE_UNLOCK(handle);
     return IMAP_NOMEM;
   }
   if(handle->timeout>0) {
@@ -712,6 +729,7 @@
     SSL *ssl = imap_create_ssl();
     if(!ssl) {
       imap_mbox_handle_set_msg(handle,"SSL context could not be created");
+      HANDLE_UNLOCK(handle);
       return IMAP_UNSECURE;
     }
     if(imap_setup_ssl(handle->sio, handle->host, ssl,
@@ -720,6 +738,7 @@
     else {
       imap_mbox_handle_set_msg(handle,"SSL negotiation failed");
       imap_handle_disconnect(handle);
+      HANDLE_UNLOCK(handle);
       return IMAP_UNSECURE;
     }
   }
@@ -732,6 +751,7 @@
     g_message("imap_mbox_connect:unexpected initial response(%d):\n%s\n",
 	      resp, handle->last_msg);
     imap_handle_disconnect(handle);
+    HANDLE_UNLOCK(handle);
     return IMAP_PROTOCOL_ERROR;
   }
   handle->can_fetch_body = 
@@ -743,6 +763,7 @@
           imap_mbox_handle_can_do(handle, IMCAP_STARTTLS)) {
     if( imap_handle_starttls(handle) != IMR_OK) {
       imap_mbox_handle_set_msg(handle,"TLS negotiation failed");
+      HANDLE_UNLOCK(handle);
       return IMAP_UNSECURE; /* TLS negotiation error */
     }
     resp = IMR_OK; /* secured with TLS */
@@ -753,8 +774,10 @@
 #endif
   if(handle->tls_mode == IMAP_TLS_REQUIRED && resp != IMR_OK) {
     imap_mbox_handle_set_msg(handle,"TLS required but not available");
+    HANDLE_UNLOCK(handle);
     return IMAP_UNSECURE;
   }
+  HANDLE_UNLOCK(handle);
   return IMAP_SUCCESS;
 }
 
@@ -860,15 +883,20 @@
                            const char *namespace)
 {
   int delim;
+  guint handler_id;
+  gchar * cmd;
+
+  HANDLE_LOCK(handle);
   /* FIXME: block other list response signals here? */
-  guint handler_id = g_signal_connect(G_OBJECT(handle), "list-response",
-                                      G_CALLBACK(get_delim),
-                                       &delim);
+  handler_id = g_signal_connect(G_OBJECT(handle), "list-response",
+				G_CALLBACK(get_delim),
+				&delim);
 
-  gchar * cmd = g_strdup_printf("LIST \"%s\" \"\"", namespace);
+  cmd = g_strdup_printf("LIST \"%s\" \"\"", namespace);
   imap_cmd_exec(handle, cmd);
   g_free(cmd);
   g_signal_handler_disconnect(G_OBJECT(handle), handler_id);
+  HANDLE_UNLOCK(handle);
   return delim;
 
 }

Modified: trunk/libbalsa/libbalsa.c
==============================================================================
--- trunk/libbalsa/libbalsa.c	(original)
+++ trunk/libbalsa/libbalsa.c	Sun Dec 21 21:27:44 2008
@@ -103,14 +103,26 @@
 
     g_mime_init(0);
 
-    /* Register our types */
-    /* So that libbalsa_mailbox_new_from_config will work... */
+    GMIME_TYPE_DATA_WRAPPER;
+    GMIME_TYPE_FILTER;
+    GMIME_TYPE_FILTER_CRLF;
+    GMIME_TYPE_PARSER;
+    GMIME_TYPE_STREAM;
+    GMIME_TYPE_STREAM_BUFFER;
+    GMIME_TYPE_STREAM_MEM;
+    GMIME_TYPE_STREAM_NULL;
+
+    /* Register our types to avoid possible race conditions. See
+       output of "valgrind --tool=helgrind --log-file=balsa.log balsa"
+       Mailbox type registration is needed also for
+       libbalsa_mailbox_new_from_config() to work. */
     LIBBALSA_TYPE_MAILBOX_LOCAL;
     LIBBALSA_TYPE_MAILBOX_POP3;
     LIBBALSA_TYPE_MAILBOX_IMAP;
     LIBBALSA_TYPE_MAILBOX_MBOX;
     LIBBALSA_TYPE_MAILBOX_MH;
     LIBBALSA_TYPE_MAILBOX_MAILDIR;
+    LIBBALSA_TYPE_MESSAGE;
 
     LIBBALSA_TYPE_ADDRESS_BOOK_VCARD;
     LIBBALSA_TYPE_ADDRESS_BOOK_EXTERN;

Modified: trunk/libbalsa/mailbox.c
==============================================================================
--- trunk/libbalsa/mailbox.c	(original)
+++ trunk/libbalsa/mailbox.c	Sun Dec 21 21:27:44 2008
@@ -1776,12 +1776,12 @@
 
     message = LIBBALSA_MAILBOX_GET_CLASS(mailbox)->get_message(mailbox,
                                                                msgno);
-    libbalsa_unlock_mailbox(mailbox);
-
     if (message && mailbox->mindex)
         /* Cache the message info, if we do not already have it. */
         lbm_cache_message(mailbox, msgno, message);
 
+    libbalsa_unlock_mailbox(mailbox);
+
     return message;
 }
 
@@ -3495,7 +3495,9 @@
             /* Prepare-threading failed--perhaps mailbox was closed. */
             return;
     }
+    libbalsa_lock_mailbox(mbox);
     lbm_sort(mbox, mbox->msg_tree);
+    libbalsa_unlock_mailbox(mbox);
 
     libbalsa_mailbox_changed(mbox);
 }

Modified: trunk/libbalsa/mailbox_imap.c
==============================================================================
--- trunk/libbalsa/mailbox_imap.c	(original)
+++ trunk/libbalsa/mailbox_imap.c	Sun Dec 21 21:27:44 2008
@@ -1922,6 +1922,7 @@
     struct message_info *msg_info;
     LibBalsaMailboxImap *mimap = (LibBalsaMailboxImap *) mailbox;
 
+    libbalsa_lock_mailbox(mailbox);
     msg_info = message_info_from_msgno(mimap, msgno);
 
     if (!msg_info->message) {
@@ -1940,6 +1941,8 @@
     }
     if (msg_info->message)
 	g_object_ref(msg_info->message); /* we want to keep one copy */
+    libbalsa_unlock_mailbox(mailbox);
+
     return msg_info->message;
 }
 
@@ -3135,8 +3138,10 @@
 libbalsa_mailbox_imap_total_messages(LibBalsaMailbox * mailbox)
 {
     LibBalsaMailboxImap *mimap = (LibBalsaMailboxImap *) mailbox;
+    guint cnt;
 
-    return mimap->messages_info ? mimap->messages_info->len : 0;
+    cnt = mimap->messages_info ? mimap->messages_info->len : 0;
+    return cnt;
 }
 
 /* Copy messages in the list to dest; use server-side copy if mailbox

Modified: trunk/libbalsa/mailbox_local.c
==============================================================================
--- trunk/libbalsa/mailbox_local.c	(original)
+++ trunk/libbalsa/mailbox_local.c	Sun Dec 21 21:27:44 2008
@@ -2165,7 +2165,7 @@
         LIBBALSA_MAILBOX_LOCAL_GET_CLASS(local)->get_info;
     guint i;
     guint changed = 0;
-
+    libbalsa_lock_mailbox(mailbox);
     for (i = 0; i < msgnos->len; i++) {
         guint msgno = g_array_index(msgnos, guint, i);
         LibBalsaMailboxLocalMessageInfo *msg_info;
@@ -2199,6 +2199,7 @@
         mailbox->unread_messages +=
             is_unread_undeleted - was_unread_undeleted;
     }
+    libbalsa_unlock_mailbox(mailbox);
 
     if (changed > 0) {
         libbalsa_mailbox_set_unread_messages_flag(mailbox,

Modified: trunk/src/balsa-app.c
==============================================================================
--- trunk/src/balsa-app.c	(original)
+++ trunk/src/balsa-app.c	Sun Dec 21 21:27:44 2008
@@ -460,9 +460,6 @@
     g_list_free(balsa_app.address_book_list);
     balsa_app.address_book_list = NULL;
 
-    /* close all mailboxes */
-    gtk_widget_destroy(balsa_app.notebook);
-
     /* now free filters */
     g_slist_foreach(balsa_app.filters, (GFunc)libbalsa_filter_free, 
 		    GINT_TO_POINTER(TRUE));

Modified: trunk/src/main.c
==============================================================================
--- trunk/src/main.c	(original)
+++ trunk/src/main.c	Sun Dec 21 21:27:44 2008
@@ -812,8 +812,6 @@
     balsa_app.main_window = BALSA_WINDOW(window);
     g_object_add_weak_pointer(G_OBJECT(window),
 			      (gpointer) &balsa_app.main_window);
-    g_signal_connect(G_OBJECT(window), "destroy",
-                     G_CALLBACK(balsa_cleanup), NULL);
 
     /* load mailboxes */
     config_load_sections();
@@ -873,6 +871,8 @@
     gdk_threads_enter();
     gtk_main();
     gdk_threads_leave();
+
+    balsa_cleanup();
     accel_map_save();
 
 #ifdef BALSA_USE_THREADS