[gnote] Fix note search by uuid and signal thread safety

[Aurimas Äernius <aurimasc src gnome org>]

commit 64c31f4eb7f3bc00bca3801602909f7fab00e785
Author: Aurimas Äernius <aurisc4 gmail com>
Date:   Mon Feb 6 23:11:30 2012 +0200

    Fix note search by uuid and signal thread safety
    
    * Fix incorrect UUID prefix, when searching for note be UUID
    * Solve deadlocks and crashes dues inproper usage of signals and locks
    * Correctly pass data to signal handlers

 src/synchronization/syncdialog.cpp  |   59 ++++++++++------------------------
 src/synchronization/syncdialog.hpp  |    4 +--
 src/synchronization/syncmanager.cpp |   10 +++---
 src/synchronization/syncui.cpp      |   25 +++++++++++++++
 src/synchronization/syncui.hpp      |    2 +
 5 files changed, 51 insertions(+), 49 deletions(-)
---
diff --git a/src/synchronization/syncdialog.cpp b/src/synchronization/syncdialog.cpp
index 2fcfe25..9bc9e79 100644
--- a/src/synchronization/syncdialog.cpp
+++ b/src/synchronization/syncdialog.cpp
@@ -69,14 +69,10 @@ void gnote_sync_dialog_class_init(GnoteSyncDialogClass *klass)
                    GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | G_SIGNAL_NO_HOOKS),
                    0, NULL, NULL, g_cclosure_marshal_VOID__INT,
                    G_TYPE_NONE, 1, G_TYPE_INT, NULL);
-  g_signal_new("note-synchronized", G_TYPE_FROM_CLASS(klass),
-                   GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | G_SIGNAL_NO_HOOKS),
-                   0, NULL, NULL, g_cclosure_marshal_generic,
-                   G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_INT, NULL);
   g_signal_new("note-conflict-detected", G_TYPE_FROM_CLASS(klass),
                    GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | G_SIGNAL_NO_HOOKS),
                    0, NULL, NULL, g_cclosure_marshal_VOID__VOID,
-                   G_TYPE_NONE, 0, NULL);
+                   G_TYPE_NONE, 1, G_TYPE_POINTER, NULL);
 }
 
 GObject *gnote_sync_dialog_new()
@@ -91,7 +87,7 @@ struct NoteConflictDetectedArgs
   NoteManager *manager;
   Note::Ptr localConflictNote;
   NoteUpdate *remoteNote;
-  const std::list<std::string> *noteUpdateTitles;
+  std::list<std::string> noteUpdateTitles;
   SyncTitleConflictResolution savedBehavior;
   SyncTitleConflictResolution resolution;
   std::exception *mainThreadException;
@@ -116,7 +112,8 @@ public:
     , m_note_update_titles(noteUpdateTitles)
     {
       // Suggest renaming note by appending " (old)" to the existing title
-      std::string suggestedRenameBase = existingNote->get_title() + _(" (old)");
+      char *old = _(" (old)");
+      std::string suggestedRenameBase = existingNote->get_title() + old;
       std::string suggestedRename = suggestedRenameBase;
       for(int i = 1; !is_note_title_available(suggestedRename); i++) {
         suggestedRename = suggestedRenameBase + " " + boost::lexical_cast<std::string>(i);
@@ -275,7 +272,6 @@ SyncDialog::SyncDialog()
 {
   m_obj = gnote_sync_dialog_new();
   g_signal_connect(m_obj, "sync-state-changed", G_CALLBACK(on_sync_state_changed), this);
-  g_signal_connect(m_obj, "note-synchronized", G_CALLBACK(on_note_synchronized), this);
   g_signal_connect(m_obj, "note-conflict-detected", G_CALLBACK(on_note_conflict_detected), this);
   m_progress_bar_timeout_id = 0;
 
@@ -618,26 +614,6 @@ void SyncDialog::sync_state_changed_(SyncState state)
 
 void SyncDialog::note_synchronized(const std::string & noteTitle, NoteSyncType type)
 {
-  // This event handler will be called by the synchronization thread
-  gdk_threads_enter();
-  g_signal_emit_by_name(m_obj, "note-synchronized", noteTitle.c_str(), static_cast<int>(type));
-  gdk_threads_leave();
-}
-
-
-void SyncDialog::on_note_synchronized(GObject*, const char * noteTitle, int type, gpointer data)
-{
-  try {
-    static_cast<SyncDialog*>(data)->note_synchronized_(noteTitle, static_cast<NoteSyncType>(type));
-  }
-  catch(...) {
-    DBG_OUT("Exception caught in %s\n", __func__);
-  }
-}
-
-
-void SyncDialog::note_synchronized_(const std::string & noteTitle, NoteSyncType type)
-{
   // FIXME: Change these strings to be more user-friendly
   // TODO: Update status for a note when status changes ("Uploading" -> "Uploaded", etc)
   std::string statusText;
@@ -670,36 +646,37 @@ void SyncDialog::note_conflict_detected(NoteManager & manager,
                                         NoteUpdate remoteNote,
                                         const std::list<std::string> & noteUpdateTitles)
 {
-  NoteConflictDetectedArgs args;
-  args.savedBehavior = CANCEL;
+  NoteConflictDetectedArgs *args = new NoteConflictDetectedArgs;
+  args->savedBehavior = CANCEL;
   int dlgBehaviorPref = Preferences::obj()
     .get_schema_settings(Preferences::SCHEMA_SYNC)->get_int(Preferences::SYNC_CONFIGURED_CONFLICT_BEHAVIOR);
   // TODO: Check range of this int
-  args.savedBehavior = static_cast<SyncTitleConflictResolution>(dlgBehaviorPref);
+  args->savedBehavior = static_cast<SyncTitleConflictResolution>(dlgBehaviorPref);
 
-  args.resolution = OVERWRITE_EXISTING;
-  args.manager = &manager;
-  args.localConflictNote = localConflictNote;
-  args.remoteNote = &remoteNote;
-  args.noteUpdateTitles = &noteUpdateTitles;
+  args->resolution = OVERWRITE_EXISTING;
+  args->manager = &manager;
+  args->localConflictNote = localConflictNote;
+  args->remoteNote = &remoteNote;
+  args->noteUpdateTitles = noteUpdateTitles;
   // This event handler will be called by the synchronization thread
   // so we have to use the delegate here to manipulate the GUI.
   // To be consistent, any exceptions in the delgate will be caught
   // and then rethrown in the synchronization thread.
   gdk_threads_enter();
-  g_signal_emit_by_name(m_obj, "note-conflict-detected", &args);
+  g_signal_emit_by_name(m_obj, "note-conflict-detected", args);
   gdk_threads_leave();
-  if(args.mainThreadException != NULL) {
-    throw *args.mainThreadException;
+  if(args->mainThreadException != NULL) {
+    std::auto_ptr<NoteConflictDetectedArgs> for_deletion(args);
+    throw *for_deletion->mainThreadException;
   }
 }
 
 
-void SyncDialog::on_note_conflict_detected(GObject*, gpointer data)
+void SyncDialog::on_note_conflict_detected(GObject*, gpointer data, gpointer)
 {
   NoteConflictDetectedArgs *args = static_cast<NoteConflictDetectedArgs*>(data);
   try {
-    SyncTitleConflictDialog conflictDlg(args->localConflictNote, *args->noteUpdateTitles);
+    SyncTitleConflictDialog conflictDlg(args->localConflictNote, args->noteUpdateTitles);
     Gtk::ResponseType reponse = Gtk::RESPONSE_OK;
 
     bool noteSyncBitsMatch = SyncManager::obj().synchronized_note_xml_matches(
diff --git a/src/synchronization/syncdialog.hpp b/src/synchronization/syncdialog.hpp
index e7c9cb0..ad19e0b 100644
--- a/src/synchronization/syncdialog.hpp
+++ b/src/synchronization/syncdialog.hpp
@@ -61,8 +61,7 @@ namespace sync {
   private:
     static void on_expander_activated(GtkExpander*, gpointer);
     static void on_sync_state_changed(GObject*, int, gpointer);
-    static void on_note_synchronized(GObject*, const char*, int, gpointer);
-    static void on_note_conflict_detected(GObject*, gpointer);
+    static void on_note_conflict_detected(GObject*, gpointer, gpointer);
     static void rename_note(const Note::Ptr & note, const std::string & newTitle, bool updateReferencingNotes);
 
     SyncDialog();
@@ -71,7 +70,6 @@ namespace sync {
     void treeview_col1_data_func(Gtk::CellRenderer *renderer, const Gtk::TreeIter & iter);
     void treeview_col2_data_func(Gtk::CellRenderer *renderer, const Gtk::TreeIter & iter);
     void sync_state_changed_(SyncState state);
-    void note_synchronized_(const std::string & noteTitle, NoteSyncType type);
 
     Gtk::Image *m_image;
     Gtk::Label *m_header_label;
diff --git a/src/synchronization/syncmanager.cpp b/src/synchronization/syncmanager.cpp
index df339d9..f1067d3 100644
--- a/src/synchronization/syncmanager.cpp
+++ b/src/synchronization/syncmanager.cpp
@@ -305,7 +305,7 @@ namespace sync {
         if(!find_note_by_uuid(iter->second.m_uuid) != 0) {
           Note::Ptr existingNote = note_mgr().find(iter->second.m_title);
           if(existingNote != 0 && !iter->second.basically_equal_to(existingNote)) {
-            // Logger.Debug ("Sync: Early conflict detection for '{0}'", noteUpdate.Title);
+            DBG_OUT("Sync: Early conflict detection for '%s'", iter->second.m_title.c_str());
             if(m_sync_ui != 0) {
               m_sync_ui->note_conflict_detected(note_mgr(), existingNote, iter->second, noteUpdateTitles);
 
@@ -387,14 +387,14 @@ namespace sync {
           (*iter)->save();
           newOrModifiedNotes.push_back(*iter);
           if(m_sync_ui != 0)
-            m_sync_ui->note_synchronized((*iter)->get_title(), UPLOAD_NEW);
+            m_sync_ui->note_synchronized_th((*iter)->get_title(), UPLOAD_NEW);
         }
         else if(m_client->get_revision(*iter) <= m_client->last_synchronized_revision()
                 && (*iter)->metadata_change_date() > m_client->last_sync_date()) {
           (*iter)->save();
           newOrModifiedNotes.push_back(*iter);
           if(m_sync_ui != 0) {
-            m_sync_ui->note_synchronized((*iter)->get_title(), UPLOAD_MODIFIED);
+            m_sync_ui->note_synchronized_th((*iter)->get_title(), UPLOAD_MODIFIED);
           }
         }
       }
@@ -418,7 +418,7 @@ namespace sync {
             if(deleted_note_titles.find(*iter) != deleted_note_titles.end()) {
               deletedTitle = deleted_note_titles[*iter];
             }
-            m_sync_ui->note_synchronized(deletedTitle, DELETE_FROM_SERVER);
+            m_sync_ui->note_synchronized_th(deletedTitle, DELETE_FROM_SERVER);
           }
         }
       }
@@ -715,7 +715,7 @@ namespace sync {
 
   Note::Ptr SyncManager::find_note_by_uuid(const std::string & uuid)
   {
-    return note_mgr().find_by_uri("note://tomboy/" + uuid);
+    return note_mgr().find_by_uri("note://gnote/" + uuid);
   }
 
 
diff --git a/src/synchronization/syncui.cpp b/src/synchronization/syncui.cpp
index 80c2aaa..ce0cc6e 100644
--- a/src/synchronization/syncui.cpp
+++ b/src/synchronization/syncui.cpp
@@ -18,6 +18,7 @@
  */
 
 
+#include "debug.hpp"
 #include "syncui.hpp"
 
 
@@ -42,6 +43,10 @@ namespace {
                  GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | G_SIGNAL_NO_HOOKS),
                  0, NULL, NULL, g_cclosure_marshal_VOID__VOID,
                  G_TYPE_NONE, 0, NULL);
+    g_signal_new("note-synchronized", G_TYPE_FROM_CLASS(klass),
+                 GSignalFlags(G_SIGNAL_RUN_LAST | G_SIGNAL_NO_RECURSE | G_SIGNAL_NO_HOOKS),
+                 0, NULL, NULL, g_cclosure_marshal_generic,
+                 G_TYPE_NONE, 2, G_TYPE_STRING, G_TYPE_INT, NULL);
   }
 
   GnoteSyncUI * gnote_sync_ui_new()
@@ -61,6 +66,7 @@ namespace sync {
   {
     g_signal_connect(m_obj, "connecting", G_CALLBACK(SyncUI::on_signal_connecting), this);
     g_signal_connect(m_obj, "idle", G_CALLBACK(SyncUI::on_signal_idle), this);
+    g_signal_connect(m_obj, "note-synchronized", G_CALLBACK(SyncUI::on_signal_note_synchronized), this);
   }
 
 
@@ -76,6 +82,25 @@ namespace sync {
   }
 
 
+  void SyncUI::on_signal_note_synchronized(GObject*, const char *noteTitle, int type, gpointer data)
+  {
+    try {
+      static_cast<SyncUI*>(data)->note_synchronized(noteTitle, static_cast<NoteSyncType>(type));
+    }
+    catch(...) {
+      DBG_OUT("Exception caught in %s\n", __func__);
+    }
+  }
+
+
+  void SyncUI::note_synchronized_th(const std::string & noteTitle, NoteSyncType type)
+  {
+    gdk_threads_enter();
+    g_signal_emit_by_name(m_obj, "note-synchronized", noteTitle.c_str(), static_cast<int>(type));
+    gdk_threads_leave();
+  }
+
+
   sigc::connection SyncUI::signal_connecting_connect(const SlotConnecting & slot)
   {
     return m_signal_connecting.connect(slot);
diff --git a/src/synchronization/syncui.hpp b/src/synchronization/syncui.hpp
index fe878ad..812da5e 100644
--- a/src/synchronization/syncui.hpp
+++ b/src/synchronization/syncui.hpp
@@ -43,6 +43,7 @@ namespace sync {
     typedef sigc::slot<void> SlotIdle;
 
     virtual void sync_state_changed(SyncState state) = 0;
+    void note_synchronized_th(const std::string & noteTitle, NoteSyncType type);
     virtual void note_synchronized(const std::string & noteTitle, NoteSyncType type) = 0;
     virtual void note_conflict_detected(NoteManager & manager,
                                         const Note::Ptr & localConflictNote,
@@ -58,6 +59,7 @@ namespace sync {
   private:
     static void on_signal_connecting(GObject*, gpointer);
     static void on_signal_idle(GObject*, gpointer);
+    static void on_signal_note_synchronized(GObject*, const char*, int, gpointer);
 
     sigc::signal<void> m_signal_connecting;
     sigc::signal<void> m_signal_idle;