[gnome-keyring] daemon: Provide caller syncronization for quitting the daemon

[Stefan Walter <stefw src gnome org>]

commit 61cc045d145e2211a5d4993b9f1f29f548a0177e
Author: Stef Walter <stefw gnome org>
Date:   Fri Mar 14 11:05:21 2014 +0100

    daemon: Provide caller syncronization for quitting the daemon
    
    Quit control messages are a bit strange because the daemon will quit
    shortly afterwards.  There are three syncronization issues here.
    
    1. We need the response to be written right away, because if
       we wait for the main loop it might not be written.
    2. Callers may want to wait for the daemon to exit, so keep the
       socket open until we do.
    3. Prevent additional connections on the control socket.

 daemon/control/gkd-control-client.c |    5 ++++
 daemon/control/gkd-control-server.c |   42 ++++++++++++++++++++++++----------
 daemon/control/gkd-control.h        |    3 ++
 daemon/gkd-main.c                   |    2 +
 pam/gkr-pam-client.c                |   33 ++++++++++++++++++++++-----
 5 files changed, 66 insertions(+), 19 deletions(-)
---
diff --git a/daemon/control/gkd-control-client.c b/daemon/control/gkd-control-client.c
index e744617..ecfdc5c 100644
--- a/daemon/control/gkd-control-client.c
+++ b/daemon/control/gkd-control-client.c
@@ -181,6 +181,11 @@ control_chat (const gchar *directory,
                return FALSE;
 
        ret = control_write (sock, buffer) && control_read (sock, buffer);
+
+       if (flags & GKD_CONTROL_WAIT_FOR_CLOSE) {
+               while (read (sock, &path, sizeof (path)) < 0);
+       }
+
        close (sock);
        return ret;
 }
diff --git a/daemon/control/gkd-control-server.c b/daemon/control/gkd-control-server.c
index 5a81eb4..65a1b04 100644
--- a/daemon/control/gkd-control-server.c
+++ b/daemon/control/gkd-control-server.c
@@ -48,6 +48,8 @@ typedef struct _ControlData {
 
 EGG_SECURE_DECLARE (control_server);
 
+static gchar *control_path;
+
 /* -----------------------------------------------------------------------------------
  * CONTROL SERVER
  */
@@ -242,11 +244,24 @@ control_process (EggBuffer *req, GIOChannel *channel)
                g_return_if_fail (!egg_buffer_has_error (&cdata->buffer));
                egg_buffer_set_uint32 (&cdata->buffer, 0, cdata->buffer.len);
 
-               /* Can't send response in main loop, send here */
+               /*
+                * Quit messages are a bit strange because the daemon will quit.
+                * There are three syncronization issues here.
+                * 1. We need the response to be written right away, because if
+                *    we wait for the main loop it might not be written.
+                * 2. Callers may want to wait for the daemon to exit, so keep the
+                *    socket open until we do.
+                * 3. Prevent additional connections on the control socket (done via
+                *    gkd_control_stop()).
+                */
                if (op == GKD_CONTROL_OP_QUIT) {
                        if (write (g_io_channel_unix_get_fd (channel),
                                   cdata->buffer.buf, cdata->buffer.len) != cdata->buffer.len)
                                g_message ("couldn't write response to close control request");
+                       if (res == GKD_CONTROL_RESULT_OK) {
+                               egg_cleanup_register ((GDestroyNotify)g_io_channel_unref,
+                                                     g_io_channel_ref (channel));
+                       }
                        control_data_free (cdata);
 
                /* Any other response, send in the main loop */
@@ -373,12 +388,14 @@ control_accept (GIOChannel *channel, GIOCondition cond, gpointer callback_data)
        return TRUE;
 }
 
-static void
-control_cleanup_channel (gpointer user_data)
+void
+gkd_control_stop (void)
 {
-       gchar *path = user_data;
-       unlink (path);
-       g_free (path);
+       if (control_path) {
+               unlink (control_path);
+               g_free (control_path);
+               control_path = NULL;
+       }
 }
 
 gboolean
@@ -386,13 +403,12 @@ gkd_control_listen (void)
 {
        struct sockaddr_un addr;
        GIOChannel *channel;
-       gchar *path;
        int sock;
 
-       path = g_strdup_printf ("%s/control", gkd_util_get_master_directory ());
-       egg_cleanup_register (control_cleanup_channel, path);
+       control_path = g_strdup_printf ("%s/control", gkd_util_get_master_directory ());
+       egg_cleanup_register ((GDestroyNotify)gkd_control_stop, NULL);
 
-       unlink (path);
+       unlink (control_path);
 
        sock = socket (AF_UNIX, SOCK_STREAM, 0);
        if (sock < 0) {
@@ -402,15 +418,15 @@ gkd_control_listen (void)
 
        memset (&addr, 0, sizeof (addr));
        addr.sun_family = AF_UNIX;
-       g_strlcpy (addr.sun_path, path, sizeof (addr.sun_path));
+       g_strlcpy (addr.sun_path, control_path, sizeof (addr.sun_path));
        if (bind (sock, (struct sockaddr*) &addr, sizeof (addr)) < 0) {
-               g_warning ("couldn't bind to control socket: %s: %s", path, g_strerror (errno));
+               g_warning ("couldn't bind to control socket: %s: %s", control_path, g_strerror (errno));
                close (sock);
                return FALSE;
        }
 
        if (listen (sock, 128) < 0) {
-               g_warning ("couldn't listen on control socket: %s: %s", path, g_strerror (errno));
+               g_warning ("couldn't listen on control socket: %s: %s", control_path, g_strerror (errno));
                close (sock);
                return FALSE;
        }
diff --git a/daemon/control/gkd-control.h b/daemon/control/gkd-control.h
index c6d31aa..53af2c6 100644
--- a/daemon/control/gkd-control.h
+++ b/daemon/control/gkd-control.h
@@ -25,10 +25,13 @@
 
 typedef enum {
        GKD_CONTROL_QUIET_IF_NO_PEER = 1 << 0,
+       GKD_CONTROL_WAIT_FOR_CLOSE = 1 << 1,
 } GkdControlFlags;
 
 gboolean          gkd_control_listen        (void);
 
+void              gkd_control_stop          (void);
+
 gchar**           gkd_control_initialize    (const gchar *directory,
                                              const gchar *components,
                                              const gchar **env);
diff --git a/daemon/gkd-main.c b/daemon/gkd-main.c
index 5c5381c..2bcfc04 100644
--- a/daemon/gkd-main.c
+++ b/daemon/gkd-main.c
@@ -399,6 +399,8 @@ dump_diagnostics (void)
 void
 gkd_main_quit (void)
 {
+       /* Always stop accepting control connections immediately */
+       gkd_control_stop ();
        g_main_loop_quit (loop);
 }
 
diff --git a/pam/gkr-pam-client.c b/pam/gkr-pam-client.c
index 5c92cec..ee17547 100644
--- a/pam/gkr-pam-client.c
+++ b/pam/gkr-pam-client.c
@@ -265,7 +265,10 @@ write_part (int fd, const unsigned char *data, int len, int *res)
 }
 
 static int 
-read_part (int fd, unsigned char *data, int len) 
+read_part (int fd,
+           unsigned char *data,
+           int len,
+           int disconnect_ok)
 {
        int r, all;
        
@@ -275,11 +278,15 @@ read_part (int fd, unsigned char *data, int len)
                if (r < 0) {
                        if (errno == EAGAIN)
                                continue;
+                       if (errno == ECONNRESET && disconnect_ok)
+                               return 0;
                        syslog (GKR_LOG_ERR, "couldn't read data from gnome-keyring-daemon: %s",
                                strerror (errno));
                        return -1;
                } 
-               if (r == 0) { 
+               if (r == 0) {
+                       if (disconnect_ok)
+                               return 0;
                        syslog (GKR_LOG_ERR, "couldn't read data from gnome-keyring-daemon: %s",
                                "unexpected end of data");
                        return -1;
@@ -300,6 +307,7 @@ keyring_daemon_op (struct sockaddr_un *addr,
 {
        int ret = GKD_CONTROL_RESULT_OK;
        unsigned char buf[4];
+       int want_disconnect;
        int i, sock = -1;
        uint oplen, l;
 
@@ -343,9 +351,14 @@ keyring_daemon_op (struct sockaddr_un *addr,
        
        if (ret != GKD_CONTROL_RESULT_OK)
                goto done;
-               
+       /*
+        * If we're asking the daemon to quit, then we expect
+        * disconnects after we send the initial request
+        */
+       want_disconnect = (op == GKD_CONTROL_OP_QUIT);
+
        /* Read the response length */
-       if (read_part (sock, buf, 4) != 4) {
+       if (read_part (sock, buf, 4, want_disconnect) != 4) {
                ret = GKD_CONTROL_RESULT_FAILED;
                goto done;
        }
@@ -358,12 +371,20 @@ keyring_daemon_op (struct sockaddr_un *addr,
                goto done;
        }
 
-       if (read_part (sock, buf, 4) != 4) {
+       if (read_part (sock, buf, 4, want_disconnect) != 4) {
                ret = GKD_CONTROL_RESULT_FAILED;
                goto done;
        }
        ret = egg_buffer_decode_uint32 (buf);
-       
+
+       /*
+        * If we asked the daemon to quit, wait for it to disconnect
+        * by waiting until the socket disconnects from the other end.
+        */
+       if (want_disconnect) {
+               while (read (sock, buf, 4) > 0);
+       }
+
 done:
        if (sock >= 0)
                close (sock);