From: Sam Thursfield Date: Wed, 15 Jun 2011 10:23:36 +0000 (+0100) Subject: Handle concurrent identity requests correctly X-Git-Tag: debian/0.0.2+20110913-1~12^2~9 X-Git-Url: http://www.project-moonshot.org/gitweb/?p=moonshot-ui.git;a=commitdiff_plain;h=f035a3a62518102602134743375f1a2740513d13 Handle concurrent identity requests correctly 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. --- diff --git a/Makefile.am b/Makefile.am index 62bd6ad..7ff1792 100644 --- a/Makefile.am +++ b/Makefile.am @@ -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 diff --git a/src/moonshot-dbus-server.vala b/src/moonshot-dbus-server.vala index ace5ba2..82e9568 100644 --- a/src/moonshot-dbus-server.vala +++ b/src/moonshot-dbus-server.vala @@ -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 index 0000000..88b642b --- /dev/null +++ b/src/moonshot-identity-request.vala @@ -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 (); + } +} diff --git a/src/moonshot-msrpc-server.vala b/src/moonshot-msrpc-server.vala index 98c659d..5aa3a6e 100644 --- a/src/moonshot-msrpc-server.vala +++ b/src/moonshot-msrpc-server.vala @@ -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 (); + } } diff --git a/src/moonshot-window.vala b/src/moonshot-window.vala index cf204fa..0b8bd83 100644 --- a/src/moonshot-window.vala +++ b/src/moonshot-window.vala @@ -23,7 +23,7 @@ class MainWindow : Window public IdCardWidget selected_id_card_widget; - private SourceFunc callback; + private Queue request_queue; private enum Columns { @@ -52,6 +52,8 @@ class MainWindow : Window public MainWindow() { + request_queue = new Queue(); + 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)