Handle concurrent identity requests correctly
authorSam Thursfield <samthursfield@codethink.co.uk>
Wed, 15 Jun 2011 10:23:36 +0000 (11:23 +0100)
committerSam Thursfield <samthursfield@codethink.co.uk>
Wed, 15 Jun 2011 10:23:36 +0000 (11:23 +0100)
Introduces the IdentityRequest class to replace the single selection
callback on the main window. A simple list of callbacks is not possible
due to limitations of Vala, and this method also allows sharing more
code between the D-Bus and MSRPC servers.

Makefile.am
src/moonshot-dbus-server.vala
src/moonshot-identity-request.vala [new file with mode: 0644]
src/moonshot-msrpc-server.vala
src/moonshot-window.vala

index 62bd6ad..7ff1792 100644 (file)
@@ -21,6 +21,7 @@ src_moonshot_SOURCES = \
         src/moonshot-idcard-widget.vala \
         src/moonshot-custom-vbox.vala \
         src/moonshot-identities-manager.vala \
+        src/moonshot-identity-request.vala \
         src/moonshot-window.vala \
         src/moonshot-password-dialog.vala \
         src/moonshot-utils.vala
index ace5ba2..82e9568 100644 (file)
@@ -33,28 +33,35 @@ public class MoonshotServer : Object {
     {
         bool has_service = false;
 
+        var request = new IdentityRequest (main_window,
+                                           nai,
+                                           password,
+                                           service);
+        request.set_source_func_callback (get_identity.callback);
+        request.execute ();
+        yield;
+
         nai_out = "";
         password_out = "";
         certificate_out = "";
 
-        main_window.set_callback (get_identity.callback);
-        yield;
+        var id_card = request.id_card;
 
-        var id_card = this.main_window.selected_id_card_widget.id_card;
-
-        foreach (string id_card_service in id_card.services)
-        {
-            if (id_card_service == service)
-                has_service = true;
-        }
+        if (id_card != null) {
+            foreach (string id_card_service in id_card.services)
+            {
+                if (id_card_service == service)
+                    has_service = true;
+            }
 
-        if (has_service)
-        {
-            nai_out = id_card.nai;
-            password_out = id_card.password;
-            certificate_out = "certificate";
+            if (has_service)
+            {
+                nai_out = id_card.nai;
+                password_out = id_card.password;
+                certificate_out = "certificate";
 
-            return true;
+                return true;
+            }
         }
 
         return false;
diff --git a/src/moonshot-identity-request.vala b/src/moonshot-identity-request.vala
new file mode 100644 (file)
index 0000000..88b642b
--- /dev/null
@@ -0,0 +1,61 @@
+delegate void ReturnIdentityCallback (IdentityRequest request);
+
+class IdentityRequest : Object {
+    public IdCard? id_card = null;
+    public bool complete = false;
+
+    private MainWindow main_window;
+    private string nai;
+    private string password;
+    private string certificate;
+
+    // Only one of these is used, we must support two types for
+    // the DBus and MSRPC servers.
+    ReturnIdentityCallback return_identity_cb = null;
+    SourceFunc source_func_cb = null;
+
+    public IdentityRequest (MainWindow                   main_window,
+                            string                       nai,
+                            string                       password,
+                            string                       certificate)
+    {
+        this.main_window = main_window;
+        this.nai = nai;
+        this.password = password;
+        this.certificate = certificate;
+    }
+
+    public void set_return_identity_callback (owned ReturnIdentityCallback cb)
+    {
+#if VALA_0_12
+        this.return_identity_cb = ((owned) cb);
+#else
+        this.return_identity_cb = (() => cb ());
+#endif
+    }
+
+    public void set_source_func_callback (owned SourceFunc cb)
+    {
+#if VALA_0_12
+        this.source_func_cb = ((owned) cb);
+#else
+        this.source_func_cb = (() => cb ());
+#endif
+    }
+
+    public void execute () {
+        main_window.select_identity (this);
+    }
+
+    public void return_identity (IdCard? id_card) {
+        this.id_card = id_card;
+        this.complete = true;
+
+        if (return_identity_cb != null)
+            return_identity_cb (this);
+        else if (source_func_cb != null)
+            source_func_cb ();
+        else
+            warn_if_reached ();
+    }
+}
index 98c659d..5aa3a6e 100644 (file)
@@ -1,50 +1,6 @@
 using Rpc;
 using MoonshotRpcInterface;
 
-/* This class is the closure when we pass execution from the RPC thread
- * to the GLib main loop thread; we need to be executing inside the main
- * loop before we can access any state or make any Gtk+ calls.
- */
-public class IdentityRequest : Object {
-    private MainWindow main_window;
-    private unowned Mutex mutex;
-    private unowned Cond cond;
-
-    internal IdCard? id_card = null;
-
-    public IdentityRequest (Gtk.Window window,
-                            Mutex _mutex,
-                            Cond _cond)
-    {
-        main_window = (MainWindow)window;
-        mutex = _mutex;
-        cond = _cond;
-    }
-
-    public bool main_loop_cb ()
-    {
-        // Execution is passed from the RPC get_identity() call to
-        // here, where we are inside the main loop thread.
-        main_window.set_callback (this.id_card_selected_cb);
-        return false;
-    }
-
-    public bool id_card_selected_cb ()
-    {
-        this.id_card = this.main_window.selected_id_card_widget.id_card;
-
-        mutex.lock ();
-        cond.signal ();
-
-        // Block the mainloop until the ID card details have been read and
-        // sent, to prevent races
-        cond.wait (mutex);
-        mutex.unlock ();
-
-        return false;
-    }
-}
-
 /* This class must be a singleton, because we use a global RPC
  * binding handle. I cannot picture a situation where more than
  * one instance of the same interface would be needed so this
@@ -80,31 +36,48 @@ public class MoonshotServer : Object {
                                               ref string password_out,
                                               ref string certificate_out)
     {
+        bool result = false;
         Mutex mutex = new Mutex ();
         Cond cond = new Cond ();
-        bool result;
 
         mutex.lock ();
 
-        IdentityRequest request = new IdentityRequest (main_window, mutex, cond);
+        var request = new IdentityRequest (main_window,
+                                           nai,
+                                           password,
+                                           service);
 
-        // Pass execution to the main loop thread and wait for
-        // the 'send' action to be signalled.
-        Idle.add (request.main_loop_cb);
-        while (request.id_card == null)
+        // Pass execution to the main loop and block the RPC thread
+        request.set_data ("mutex", mutex);
+        request.set_data ("cond", cond);
+        request.set_return_identity_callback (this.return_identity_cb);
+        Idle.add (request.execute);
+
+        while (request.complete == false)
             cond.wait (mutex);
 
-        // Send back the results. Memory is freed by the RPC runtime.
-        if (request.id_card.nai == nai || request.id_card.password == password)
-        {
-            nai_out = request.id_card.nai;
-            password_out = request.id_card.password;
-            certificate_out = "certificate";
-            result = true;
-        }
-        else
-        {
-            result = false;
+        nai_out = "";
+        password_out = "";
+        certificate_out = "";
+
+        var id_card = request.id_card;
+
+        if (id_card == null) {
+            foreach (string id_card_service in id_card.services)
+            {
+                if (id_card_service == service)
+                    has_service = true;
+            }
+
+            if (has_service)
+            {
+                // The strings are freed by the RPC runtime
+                nai_out = id_card.nai;
+                password_out = id_card.password;
+                certificate_out = "certificate";
+
+                result = true;
+            }
         }
 
         // The outputs must be set before this function is called. For this
@@ -116,4 +89,20 @@ public class MoonshotServer : Object {
         cond.signal ();
         mutex.unlock ();
     }
+
+    // Called from the main loop thread when an identity has
+    // been selected
+    public void return_identity_cb (IdentityRequest request) {
+        Mutex mutex = request.get_data ("mutex");
+        Cond cond = request.get_data ("cond");
+
+        // Notify the RPC thread that the request is complete
+        mutex.lock ();
+        cond.signal ();
+
+        // Block the main loop until the RPC call has returned
+        // to avoid any races
+        cond.wait (mutex);
+        mutex.unlock ();
+    }
 }
index cf204fa..0b8bd83 100644 (file)
@@ -23,7 +23,7 @@ class MainWindow : Window
 
     public IdCardWidget selected_id_card_widget;
 
-    private SourceFunc callback;
+    private Queue<IdentityRequest> request_queue;
 
     private enum Columns
     {
@@ -52,6 +52,8 @@ class MainWindow : Window
 
     public MainWindow()
     {
+        request_queue = new Queue<IdentityRequest>();
+
         this.title = "Moonshoot";
         this.set_position (WindowPosition.CENTER);
         set_default_size (WINDOW_WIDTH, WINDOW_HEIGHT);
@@ -338,41 +340,42 @@ class MainWindow : Window
         dialog.destroy ();
     }
 
-    public void set_callback (owned SourceFunc callback)
+    public void select_identity (IdentityRequest request)
     {
-#if VALA_0_12
-        this.callback = (owned) callback;
-#else
-        this.callback = () => callback ();
-#endif
+        /* Automatic service matching rules can go here */
+
+        /* Resort to manual selection */
+        this.request_queue.push_tail (request);
+        this.show ();
     }
 
     public void send_identity_cb (IdCardWidget id_card_widget)
     {
+        var request = this.request_queue.pop_head ();
+        var identity = id_card_widget.id_card;
         this.selected_id_card_widget = id_card_widget;
 
-        if (id_card_widget.id_card.password == null)
+        if (identity.password == null)
         {
             var dialog = new AddPasswordDialog ();
             var result = dialog.run ();
 
             switch (result) {
             case ResponseType.OK:
-                selected_id_card_widget.id_card.password = dialog.password;
-                this.hide ();
-                this.callback ();
+                identity.password = dialog.password;
                 break;
             default:
-                this.hide ();
+                identity = null;
                 break;
             }
+
             dialog.destroy ();
         }
-        else
-        {
-          this.hide ();
-          this.callback ();
-        }
+
+        if (this.request_queue.is_empty())
+            this.hide ();
+
+        request.return_identity (identity);
     }
 
     private void label_make_bold (Label label)