From bdfb1a807e03ce4a8d5456a516cbcedd837d8dd9 Mon Sep 17 00:00:00 2001
From: Stephen Gallagher <sgallagh@redhat.com>
Date: Wed, 10 Mar 2010 15:27:59 -0500
Subject: [PATCH] Automatically retry SBUS requests within the timeout period

This patch adds a new helper function, sbus_conn_send() that will
handle automatically resending a request if the connection is
closed at the time of the initial send.
---
 src/monitor/monitor.c               |   64 ++-----------
 src/monitor/monitor_sbus.c          |   34 +------
 src/providers/dp_auth_util.c        |   31 +------
 src/responder/common/responder_dp.c |    5 +
 src/responder/pam/pamsrv_dp.c       |   24 +-----
 src/sbus/sssd_dbus.h                |   24 +++++
 src/sbus/sssd_dbus_connection.c     |  171 +++++++++++++++++++++++++++++++++++
 7 files changed, 222 insertions(+), 131 deletions(-)

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 56a74fb..eda3c05 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -640,7 +640,6 @@ static int monitor_kill_service (struct mt_svc *svc)
 static void reload_reply(DBusPendingCall *pending, void *data)
 {
     DBusMessage *reply;
-    struct mt_svc *svc = talloc_get_type(data, struct mt_svc);
 
     reply = dbus_pending_call_steal_reply(pending);
     if (!reply) {
@@ -648,18 +647,14 @@ static void reload_reply(DBusPendingCall *pending, void *data)
          * until reply is valid or timeout has occurred. If reply is NULL
          * here, something is seriously wrong and we should bail out.
          */
-        DEBUG(0, ("A reply callback was called but no reply was received"
-                  " and no timeout occurred\n"));
+        DEBUG(1, ("Timeout occurred\n"));
 
-        /* Destroy this connection */
-        sbus_disconnect(svc->conn);
-        goto done;
     }
 
     /* TODO: Handle cases where the call has timed out or returned
      * with an error.
      */
-done:
+
     dbus_pending_call_unref(pending);
     dbus_message_unref(reply);
 }
@@ -687,9 +682,6 @@ static int monitor_update_resolv(struct config_file_ctx *file_ctx,
 static int service_signal(struct mt_svc *svc, const char *svc_signal)
 {
     DBusMessage *msg;
-    dbus_bool_t dbret;
-    DBusConnection *dbus_conn;
-    DBusPendingCall *pending_reply;
 
     if (svc->provider && strcasecmp(svc->provider, "local") == 0) {
         /* The local provider requires no signaling */
@@ -705,7 +697,6 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal)
         return EIO;
     }
 
-    dbus_conn = sbus_get_connection(svc->conn);
     msg = dbus_message_new_method_call(NULL,
                                        MONITOR_PATH,
                                        MONITOR_INTERFACE,
@@ -717,21 +708,9 @@ static int service_signal(struct mt_svc *svc, const char *svc_signal)
         return ENOMEM;
     }
 
-    dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                            svc->mt_ctx->service_id_timeout);
-    if (!dbret || pending_reply == NULL) {
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        monitor_kill_service(svc);
-        talloc_free(svc);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, reload_reply, svc, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(svc->conn, msg, reload_reply,
+                          svc->mt_ctx->service_id_timeout,
+                          svc, NULL);
 }
 
 static int service_signal_dns_reload(struct mt_svc *svc)
@@ -1859,9 +1838,6 @@ static int monitor_service_init(struct sbus_connection *conn, void *data)
 static int service_send_ping(struct mt_svc *svc)
 {
     DBusMessage *msg;
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
-    dbus_bool_t dbret;
 
     if (!svc->conn) {
         DEBUG(8, ("Service not yet initialized\n"));
@@ -1870,8 +1846,6 @@ static int service_send_ping(struct mt_svc *svc)
 
     DEBUG(4,("Pinging %s\n", svc->name));
 
-    dbus_conn = sbus_get_connection(svc->conn);
-
     /*
      * Set up identity request
      * This should be a well-known path and method
@@ -1887,24 +1861,9 @@ static int service_send_ping(struct mt_svc *svc)
         return ENOMEM;
     }
 
-    dbret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                            svc->mt_ctx->service_id_timeout);
-    if (!dbret || pending_reply == NULL) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        talloc_zfree(svc->conn);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, ping_check, svc, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(svc->conn, msg, ping_check,
+                   svc->mt_ctx->service_id_timeout,
+                   svc, NULL);
 }
 
 static void ping_check(DBusPendingCall *pending, void *data)
@@ -1918,12 +1877,9 @@ static void ping_check(DBusPendingCall *pending, void *data)
 
     reply = dbus_pending_call_steal_reply(pending);
     if (!reply) {
-        /* reply should never be null. This function shouldn't be called
-         * until reply is valid or timeout has occurred. If reply is NULL
-         * here, something is seriously wrong and we should bail out.
+        /* A NULL reply means an internal timeout occurred (not D-BUS)
          */
-        DEBUG(0, ("A reply callback was called but no reply was received"
-                  " and no timeout occurred\n"));
+        DEBUG(1, ("Timeout occurred\n"));
 
         /* Destroy this connection */
         sbus_disconnect(svc->conn);
diff --git a/src/monitor/monitor_sbus.c b/src/monitor/monitor_sbus.c
index d60a087..0173c19 100644
--- a/src/monitor/monitor_sbus.c
+++ b/src/monitor/monitor_sbus.c
@@ -56,14 +56,10 @@ static void id_callback(DBusPendingCall *pending, void *ptr)
 
     reply = dbus_pending_call_steal_reply(pending);
     if (!reply) {
-        /* reply should never be null. This function shouldn't be called
-         * until reply is valid or timeout has occurred. If reply is NULL
-         * here, something is seriously wrong and we should bail out.
+        /* If reply is null, a timeout occurred in SBUS code
+         * (not D-BUS)
          */
-        DEBUG(0, ("Severe error. A reply callback was called but no"
-                  " reply was received and no timeout occurred\n"));
-
-        /* FIXME: Destroy this connection ? */
+        DEBUG(0, ("Timeout occurred sending ID\n"));
         goto done;
     }
 
@@ -109,13 +105,9 @@ done:
 int monitor_common_send_id(struct sbus_connection *conn,
                            const char *name, uint16_t version)
 {
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     DBusMessage *msg;
     dbus_bool_t ret;
 
-    dbus_conn = sbus_get_connection(conn);
-
     /* create the message */
     msg = dbus_message_new_method_call(NULL,
                                        MON_SRV_PATH,
@@ -137,24 +129,8 @@ int monitor_common_send_id(struct sbus_connection *conn,
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                          30000 /* TODO: set timeout */);
-    if (!ret || !pending_reply) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(conn, msg, id_callback,
+                          3000, NULL, NULL);
 }
 
 int monitor_common_pong(DBusMessage *message,
diff --git a/src/providers/dp_auth_util.c b/src/providers/dp_auth_util.c
index 5291176..46565f8 100644
--- a/src/providers/dp_auth_util.c
+++ b/src/providers/dp_auth_util.c
@@ -283,12 +283,9 @@ static void id_callback(DBusPendingCall *pending, void *ptr)
 
     reply = dbus_pending_call_steal_reply(pending);
     if (!reply) {
-        /* reply should never be null. This function shouldn't be called
-         * until reply is valid or timeout has occurred. If reply is NULL
-         * here, something is seriously wrong and we should bail out.
+        /*  A NULL reply means an internal timeout occurred (not D-BUS)
          */
-        DEBUG(0, ("Severe error. A reply callback was called but no"
-                  " reply was received and no timeout occurred\n"));
+        DEBUG(1, ("Timeout occurred\n"));
 
         /* FIXME: Destroy this connection ? */
         goto done;
@@ -336,13 +333,9 @@ done:
 int dp_common_send_id(struct sbus_connection *conn, uint16_t version,
                       const char *name)
 {
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     DBusMessage *msg;
     dbus_bool_t ret;
 
-    dbus_conn = sbus_get_connection(conn);
-
     /* create the message */
     msg = dbus_message_new_method_call(NULL,
                                        DP_PATH,
@@ -365,23 +358,7 @@ int dp_common_send_id(struct sbus_connection *conn, uint16_t version,
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply,
-                                          30000 /* TODO: set timeout */);
-    if (!ret || !pending_reply) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    /* Set up the reply handler */
-    dbus_pending_call_set_notify(pending_reply, id_callback, NULL, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(conn, msg, id_callback, 30000,
+                          NULL, NULL);
 }
 
diff --git a/src/responder/common/responder_dp.c b/src/responder/common/responder_dp.c
index d5a4ca7..2f842c6 100644
--- a/src/responder/common/responder_dp.c
+++ b/src/responder/common/responder_dp.c
@@ -457,6 +457,11 @@ static int sss_dp_send_acct_req_create(struct resp_ctx *rctx,
         return EIO;
     }
 
+    /* Don't use sbus_conn_send here, because we need
+     * to guarantee a pending_reply pointer in order
+     * to cancel pending requests if the destructors
+     * are called.
+     */
     dbret = dbus_connection_send_with_reply(dbus_conn, msg,
                                             &pending_reply, timeout);
     if (!dbret || pending_reply == NULL) {
diff --git a/src/responder/pam/pamsrv_dp.c b/src/responder/pam/pamsrv_dp.c
index 071d09b..ff34372 100644
--- a/src/responder/pam/pamsrv_dp.c
+++ b/src/responder/pam/pamsrv_dp.c
@@ -46,7 +46,7 @@ static void pam_dp_process_reply(DBusPendingCall *pending, void *ptr)
     dbus_pending_call_block(pending);
     msg = dbus_pending_call_steal_reply(pending);
     if (msg == NULL) {
-        DEBUG(0, ("Severe error. A reply callback was called but no reply was received and no timeout occurred\n"));
+        DEBUG(0, ("Timeout occurred\n"));
         preq->pd->pam_status = PAM_SYSTEM_ERR;
         goto done;
     }
@@ -84,8 +84,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
     struct pam_data *pd = preq->pd;
     struct be_conn *be_conn;
     DBusMessage *msg;
-    DBusPendingCall *pending_reply;
-    DBusConnection *dbus_conn;
     dbus_bool_t ret;
     int res;
 
@@ -100,7 +98,6 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
                   " This maybe a bug, it shouldn't happen!\n", preq->domain));
         return EIO;
     }
-    dbus_conn = sbus_get_connection(be_conn->conn);
 
     msg = dbus_message_new_method_call(NULL,
                                        DP_PATH,
@@ -121,22 +118,7 @@ int pam_dp_send_req(struct pam_auth_req *preq, int timeout)
         return EIO;
     }
 
-    ret = dbus_connection_send_with_reply(dbus_conn, msg, &pending_reply, timeout);
-    if (!ret || pending_reply == NULL) {
-        /*
-         * Critical Failure
-         * We can't communicate on this connection
-         * We'll drop it using the default destructor.
-         */
-        DEBUG(0, ("D-BUS send failed.\n"));
-        dbus_message_unref(msg);
-        return EIO;
-    }
-
-    dbus_pending_call_set_notify(pending_reply,
-                                 pam_dp_process_reply, preq, NULL);
-    dbus_message_unref(msg);
-
-    return EOK;
+    return sbus_conn_send(be_conn->conn, msg, pam_dp_process_reply,
+                          timeout, preq, NULL);
 }
 
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index ac02c44..080cdec 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -144,6 +144,30 @@ DBusHandlerResult sbus_message_handler(DBusConnection *conn,
                                   DBusMessage *message,
                                   void *user_data);
 
+/*
+ * Send a message across the SBUS
+ *
+ * If the connection is disconnected when attempting to
+ * send, sbus_conn_send will attempt to resend the message
+ * every 50 milliseconds until the timeout passes.
+ *
+ * This is done because the sbus reconnection service may
+ * bring the connection back to life.
+ *
+ * If retry_event is not NULL, sbus_conn_send() will return
+ * a pointer to the retry event, if one is created.
+ *
+ * If waiting for a retry is unacceptable for a client, they
+ * can free this event to handle retries (if desired) on
+ * their own.
+ */
+int sbus_conn_send(struct sbus_connection *conn,
+                   DBusMessage *msg,
+                   DBusPendingCallNotifyFunction reply_handler,
+                   int timeout_ms,
+                   void *pvt,
+                   struct tevent_timer **retry_event);
+
 void sbus_conn_send_reply(struct sbus_connection *conn,
                           DBusMessage *reply);
 
diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
index 38ccc6a..17185ec 100644
--- a/src/sbus/sssd_dbus_connection.c
+++ b/src/sbus/sssd_dbus_connection.c
@@ -685,6 +685,177 @@ bool sbus_conn_disconnecting(struct sbus_connection *conn)
     return false;
 }
 
+struct sbus_conn_send_ctx {
+    struct sbus_connection *conn;
+    DBusMessage *msg;
+    DBusPendingCallNotifyFunction reply_handler;
+    int timeout_ms;
+    void *pvt;
+};
+
+static void sbus_conn_queue_send_done(struct tevent_context *ev,
+                                      struct tevent_timer *te,
+                                      struct timeval current_time,
+                                      void *pvt);
+static void sbus_conn_queue_timeout_done(struct tevent_context *ev,
+                                         struct tevent_timer *te,
+                                         struct timeval current_time,
+                                         void *pvt);
+
+/*
+ * Send a message across the SBUS
+ *
+ * If the connection is disconnected when attempting to
+ * send, sbus_conn_send will attempt to resend the message
+ * every 50 milliseconds until the timeout passes.
+ *
+ * This is done because the sbus reconnection service may
+ * bring the connection back to life.
+ *
+ * If retry_event is not NULL, sbus_conn_send() will return
+ * a pointer to the retry event, if one is created.
+ *
+ * If waiting for a retry is unacceptable for a client, they
+ * can free this event to handle retries (if desired) on
+ * their own.
+ */
+int sbus_conn_send(struct sbus_connection *conn,
+                   DBusMessage *msg,
+                   DBusPendingCallNotifyFunction reply_handler,
+                   int timeout_ms,
+                   void *pvt,
+                   struct tevent_timer **retry_event)
+{
+    DBusPendingCall *pending_reply;
+    DBusConnection *dbus_conn;
+    dbus_bool_t dbret;
+    struct sbus_conn_send_ctx *send_ctx = NULL;
+    struct tevent_timer *send_event;
+    struct timeval tv;
+    struct tevent_timer *timeout_event;
+
+    dbus_conn = sbus_get_connection(conn);
+
+    dbret = dbus_connection_send_with_reply(dbus_conn, msg,
+                                            &pending_reply,
+                                            timeout_ms);
+    if (!dbret) {
+        /*
+         * Critical Failure
+         * Insufficient memory to send message
+         */
+        DEBUG(0, ("D-BUS send failed.\n"));
+        return ENOMEM;
+    }
+
+    if (pending_reply) {
+        /* Set up the reply handler */
+        dbret = dbus_pending_call_set_notify(pending_reply, reply_handler,
+                                             pvt, NULL);
+        if (!dbret) {
+            /*
+             * Critical Failure
+             * Insufficient memory to create pending call notify
+             */
+            DEBUG(0, ("D-BUS send failed.\n"));
+            dbus_pending_call_cancel(pending_reply);
+            dbus_pending_call_unref(pending_reply);
+            return ENOMEM;
+        }
+
+        return EOK;
+    }
+
+    /* If pending_reply is NULL, the connection was not
+     * open for sending. Queue the message and try resending
+     * until the timeout passes.
+     */
+    else {
+        send_ctx = talloc_zero(conn, struct sbus_conn_send_ctx);
+        if (!send_ctx) {
+            return ENOMEM;
+        }
+
+        send_ctx->conn = conn;
+        send_ctx->msg = msg;
+        send_ctx->reply_handler = reply_handler;
+        send_ctx->timeout_ms = timeout_ms;
+        send_ctx->pvt = pvt;
+
+        /* Retry sending every 50ms until it goes through
+         * or fails completely
+         */
+        send_ctx->timeout_ms -= 50;
+
+        if (send_ctx->timeout_ms <= 0) {
+            tv = tevent_timeval_current();
+            timeout_event = tevent_add_timer(send_ctx->conn->ev, send_ctx,
+                                             tv, sbus_conn_queue_timeout_done,
+                                             send_ctx);
+            if (!timeout_event) {
+                talloc_zfree(send_ctx);
+                return ENOMEM;
+            }
+            return ETIMEDOUT;
+        }
+
+        tv = tevent_timeval_current_ofs(0, 50000);
+        send_event = tevent_add_timer(send_ctx->conn->ev, send_ctx,
+                                      tv, sbus_conn_queue_send_done,
+                                      send_ctx);
+        if (!send_event) {
+            talloc_zfree(send_ctx);
+            return EIO;
+        }
+
+        if (retry_event) {
+            *retry_event = send_event;
+        }
+
+        return EOK;
+    }
+}
+
+static void sbus_conn_queue_send_done(struct tevent_context *ev,
+                                      struct tevent_timer *te,
+                                      struct timeval current_time,
+                                      void *pvt)
+{
+    struct sbus_conn_send_ctx *send_ctx;
+
+    send_ctx = talloc_get_type(pvt, struct sbus_conn_send_ctx);
+    if(!send_ctx) {
+        /* Can't do anything without a context. */
+        return;
+    }
+
+    sbus_conn_send(send_ctx->conn,
+                   send_ctx->msg,
+                   send_ctx->reply_handler,
+                   send_ctx->timeout_ms,
+                   send_ctx->pvt,
+                   NULL);
+
+    talloc_zfree(send_ctx);
+}
+
+static void sbus_conn_queue_timeout_done(struct tevent_context *ev,
+                                         struct tevent_timer *te,
+                                         struct timeval current_time,
+                                         void *pvt)
+{
+    struct sbus_conn_send_ctx *send_ctx;
+
+    send_ctx = talloc_get_type(pvt, struct sbus_conn_send_ctx);
+    if(!send_ctx) {
+        /* Can't do anything without a context. */
+        return;
+    }
+
+    send_ctx->reply_handler(NULL, send_ctx->pvt);
+}
+
+
 void sbus_conn_send_reply(struct sbus_connection *conn, DBusMessage *reply)
 {
     dbus_connection_send(conn->dbus.conn, reply, NULL);
-- 
1.6.6.1

