RESPONDER: Shutdown {dbus,socket}-activated responders in case they're idle

This commit introduces a new option for the responders called
responder_idle_timeout, which specifies the number of seconds that the
responder process can be up without being used. The default value is
300 seconds (5 minutes) and can be configured per responder, being 60
seconds the minimum acceptable value.

Is important to note that setting "responder_idle_timeout = 0" disables
the responder timeout, which makes sense for the responders that always
will be running.

The shutdown timeout is activated per responder in case the responder
has been {dbus,socket}-activated. In case of any commnunication with the
responder the timeout is reset thereby ensuring we won't shutdown a
responder that is not idle.

Setting the responder's last request time is done slightly differently
for socket-activated and dbus-activated responders. In both cases it's
updated in any internal communication in sbus_message_handler(), but
for the socket-activated responders it's also updated when the
responder's socket is used.

Currently it works properly with all responders but the secrets one,
which has a different logic and must be treated separately in case some
change is required there.

Is worth to mention that this commit does not affect the responders
explicitly configured in the "services" line of sssd.conf.

Related:
https://fedorahosted.org/sssd/ticket/3245

Signed-off-by: Fabiano Fidêncio <fidencio@redhat.com>
Reviewed-by: Pavel Březina <pbrezina@redhat.com>
Reviewed-by: Jakub Hrozek <jhrozek@redhat.com>
Reviewed-by: Lukáš Slebodník <lslebodn@redhat.com>

1 files changed, 62 insertions, 1 deletions

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 60b9dc2e0..45700f447 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -173,6 +173,8 @@ static int start_service(struct mt_svc *mt_svc);
 
 static int monitor_service_init(struct sbus_connection *conn, void *data);
 
+static int monitor_service_shutdown(struct sbus_connection *conn, void *data);
+
 static int service_signal_reset_offline(struct mt_svc *svc);
 
 static int get_service_config(struct mt_ctx *ctx, const char *name,
@@ -304,6 +306,12 @@ static int get_service_in_the_list(struct mon_init_conn *mini,
 #endif
 }
 
+static int sbus_connection_destructor(struct sbus_connection *conn)
+{
+    return monitor_service_shutdown(conn,
+                                    sbus_connection_get_destructor_data(conn));
+}
+
 /* registers a new client.
  * if operation is successful also sends back the Monitor version */
 static int client_registration(struct sbus_request *dbus_req, void *data)
@@ -365,6 +373,15 @@ static int client_registration(struct sbus_request *dbus_req, void *data)
     /* Fill in svc structure with connection data */
     svc->conn = mini->conn;
 
+    /* For {dbus,socket}-activated services we will have to unregister then
+     * when the sbus_connection is freed. That's the reason we have to
+     * hook up on its destructor function, do the service unregistration
+     * from there and set the destructor back to NULL just before freeing
+     * the service itself. */
+    if (svc->socket_activated) {
+        talloc_set_destructor(svc->conn, sbus_connection_destructor);
+    }
+
     ret = mark_service_as_started(svc);
     if (ret) {
         DEBUG(SSSDBG_CRIT_FAILURE, "Failed to mark service [%s]!\n", svc_name);
@@ -644,7 +661,7 @@ static int monitor_dbus_init(struct mt_ctx *ctx)
      * lose any access.
      */
     ret = sbus_new_server(ctx, ctx->ev, monitor_address, ctx->uid, ctx->gid,
-                          false, &ctx->sbus_srv, monitor_service_init, ctx, NULL);
+                          false, &ctx->sbus_srv, monitor_service_init, ctx, ctx);
 
     talloc_free(monitor_address);
 
@@ -1434,6 +1451,13 @@ static void monitor_quit(struct mt_ctx *mt_ctx, int ret)
 
     /* Kill all of our known children manually */
     DLIST_FOR_EACH(svc, mt_ctx->svc_list) {
+        if (svc->socket_activated && svc->conn != NULL) {
+            /* Unset the sbus_connection destructor used to
+             * unregister the service from the monitor as
+             * it may lead to a double-free here. */
+            talloc_set_destructor(svc->conn, NULL);
+        }
+
         if (svc->pid == 0) {
             /* The local provider has no PID */
             continue;
@@ -2426,6 +2450,43 @@ static int monitor_service_init(struct sbus_connection *conn, void *data)
                                     MON_SRV_PATH, mini);
 }
 
+/*
+ * monitor_service_shutdown
+ * Unregister the client when it's connection is finished.
+ * Shuts down, from the monitor point of view, the service that just finished.
+ */
+static int monitor_service_shutdown(struct sbus_connection *conn, void *data)
+{
+    struct mt_ctx *ctx;
+    struct mt_svc *svc;
+
+    ctx = talloc_get_type(data, struct mt_ctx);
+
+    for (svc = ctx->svc_list; svc != NULL; svc = svc->next) {
+        if (svc->conn == conn) {
+            break;
+        }
+    }
+
+    if (svc != NULL) {
+        /* We must decrease the number of services when shutting down
+         * a {socket,dbus}-activted service. */
+        ctx->num_services--;
+
+        DEBUG(SSSDBG_TRACE_FUNC,
+              "Unregistering service %s (%p)\n", svc->identity, svc);
+
+        /* Before free'ing the service, let's unset the sbus_connection
+         * destructor that triggered this call, otherwise we may end up
+         * with a double-free due to a cycling call */
+        talloc_set_destructor(svc->conn, NULL);
+
+        talloc_zfree(svc);
+    }
+
+    return 0;
+}
+
 static void service_startup_handler(struct tevent_context *ev,
                                     struct tevent_timer *te,
                                     struct timeval t, void *ptr);