SBUS: Add destructor data to sbus_connection

This additions has a very specific reason: unregister a service when
it's shutdown.

So far, we never had to do this kind of operation because the services
were started during SSSD's startup when finished when SSSD finished.

Now, with the socket-activation in place the game will be a little bit
different as the services will have an idle timeout and will be able
shut themselves down. In order to do it properly the monitor will need
to "unregister" the service and there's no way to do that without adding
this destructor data to the sbus_connection structure and introducing a
new function to access it from the monitor (where we're going to
set the destructor function to the sbus_connection for the
socket-activated services).

So far it's not being used anywhere as every function taking it as
parameter is just receiving NULL, but it will be used in the follow up
commits, by the monitor.

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>

9 files changed, 59 insertions, 21 deletions

diff --git a/src/monitor/monitor.c b/src/monitor/monitor.c
index 30cd3f1a6..60b9dc2e0 100644
--- a/src/monitor/monitor.c
+++ b/src/monitor/monitor.c
@@ -644,7 +644,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);
+                          false, &ctx->sbus_srv, monitor_service_init, ctx, NULL);
 
     talloc_free(monitor_address);
 
diff --git a/src/providers/data_provider/dp.c b/src/providers/data_provider/dp.c
index b672370aa..bab47f4d7 100644
--- a/src/providers/data_provider/dp.c
+++ b/src/providers/data_provider/dp.c
@@ -42,7 +42,7 @@ static errno_t dp_init_dbus_server(struct data_provider *provider)
     ret = sbus_new_server(provider, provider->ev, sbus_address,
                           provider->uid, provider->gid, true,
                           &provider->srv_conn,
-                          dp_client_init, provider);
+                          dp_client_init, provider, NULL);
     talloc_free(sbus_address);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Could not set up sbus server.\n");
diff --git a/src/providers/proxy/proxy_init.c b/src/providers/proxy/proxy_init.c
index 2241dafb8..7c9d3dafb 100644
--- a/src/providers/proxy/proxy_init.c
+++ b/src/providers/proxy/proxy_init.c
@@ -179,7 +179,7 @@ static errno_t proxy_setup_sbus(TALLOC_CTX *mem_ctx,
     }
 
     ret = sbus_new_server(mem_ctx, be_ctx->ev, sbus_address, 0, be_ctx->gid,
-                          false, &ctx->sbus_srv, proxy_client_init, ctx);
+                          false, &ctx->sbus_srv, proxy_client_init, ctx, NULL);
     talloc_free(sbus_address);
     if (ret != EOK) {
         DEBUG(SSSDBG_FATAL_FAILURE, "Could not set up sbus server.\n");
diff --git a/src/responder/ifp/ifpsrv.c b/src/responder/ifp/ifpsrv.c
index 37b62a7f4..0dc61a422 100644
--- a/src/responder/ifp/ifpsrv.c
+++ b/src/responder/ifp/ifpsrv.c
@@ -138,7 +138,7 @@ sysbus_init(TALLOC_CTX *mem_ctx,
     /* Integrate with tevent loop */
     ret = sbus_init_connection(system_bus, ev, conn,
                                SBUS_CONN_TYPE_SYSBUS,
-                               NULL, &system_bus->conn);
+                               NULL, NULL, &system_bus->conn);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE,
               "Could not integrate D-BUS into mainloop.\n");
diff --git a/src/sbus/sssd_dbus.h b/src/sbus/sssd_dbus.h
index 0cce8b963..5a66f09d5 100644
--- a/src/sbus/sssd_dbus.h
+++ b/src/sbus/sssd_dbus.h
@@ -74,12 +74,6 @@ struct sbus_request;
 typedef int (*sbus_msg_handler_fn)(struct sbus_request *dbus_req,
                                    void *handler_data);
 
-/*
- * sbus_conn_destructor_fn
- * Function to be called when a connection is finalized
- */
-typedef int (*sbus_conn_destructor_fn)(void *);
-
 typedef void (*sbus_conn_reconn_callback_fn)(struct sbus_connection *, int, void *);
 
 /*
@@ -141,7 +135,9 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
                     uid_t uid, gid_t gid,
                     bool use_symlink,
                     struct sbus_connection **server,
-                    sbus_server_conn_init_fn init_fn, void *init_pvt_data);
+                    sbus_server_conn_init_fn init_fn,
+                    void *init_pvt_data,
+                    void *client_destructor_data);
 
 /* Connection Functions */
 
@@ -175,6 +171,7 @@ int sbus_init_connection(TALLOC_CTX *ctx,
                          DBusConnection *dbus_conn,
                          int connection_type,
                          time_t *last_request_time,
+                         void *destructor_data,
                          struct sbus_connection **_conn);
 
 DBusConnection *sbus_get_connection(struct sbus_connection *conn);
@@ -450,4 +447,10 @@ sbus_signal_listen(struct sbus_connection *conn,
                    sbus_incoming_signal_fn handler_fn,
                    void *handler_data);
 
+/* This function returns the destructor data passed when in starting
+ * a new dbus server/connection. Its use, for now, must be restricted
+ * to {dbus,socket}-activated services in order to proper shut them
+ * down, unregistering them in the monitor. */
+void *sbus_connection_get_destructor_data(struct sbus_connection *conn);
+
 #endif /* _SSSD_DBUS_H_*/
diff --git a/src/sbus/sssd_dbus_connection.c b/src/sbus/sssd_dbus_connection.c
index 2536ef630..450cee904 100644
--- a/src/sbus/sssd_dbus_connection.c
+++ b/src/sbus/sssd_dbus_connection.c
@@ -149,6 +149,7 @@ int sbus_init_connection(TALLOC_CTX *ctx,
                          DBusConnection *dbus_conn,
                          int connection_type,
                          time_t *last_request_time,
+                         void *client_destructor_data,
                          struct sbus_connection **_conn)
 {
     struct sbus_connection *conn;
@@ -163,6 +164,7 @@ int sbus_init_connection(TALLOC_CTX *ctx,
     conn->dbus.conn = dbus_conn;
     conn->connection_type = connection_type;
     conn->last_request_time = last_request_time;
+    conn->client_destructor_data = client_destructor_data;
 
     ret = sbus_opath_hash_init(conn, conn, &conn->managed_paths);
     if (ret != EOK) {
@@ -282,7 +284,7 @@ int sbus_new_connection(TALLOC_CTX *ctx, struct tevent_context *ev,
     }
 
     ret = sbus_init_connection(ctx, ev, dbus_conn, SBUS_CONN_TYPE_SHARED,
-                               last_request_time, &conn);
+                               last_request_time, NULL, &conn);
     if (ret != EOK) {
         /* FIXME: release resources */
     }
@@ -610,3 +612,13 @@ void sbus_allow_uid(struct sbus_connection *conn, uid_t *uid)
                                            is_uid_sssd_user,
                                            uid, NULL);
 }
+
+void *sbus_connection_get_destructor_data(struct sbus_connection *conn)
+{
+    if (conn == NULL) {
+        /* Should never happen! */
+        return NULL;
+    }
+
+    return conn->client_destructor_data;
+}
diff --git a/src/sbus/sssd_dbus_private.h b/src/sbus/sssd_dbus_private.h
index cdbf1d584..8abca66b0 100644
--- a/src/sbus/sssd_dbus_private.h
+++ b/src/sbus/sssd_dbus_private.h
@@ -69,6 +69,9 @@ struct sbus_connection {
 
     /* responder related stuff */
     time_t *last_request_time;
+
+    /* client related stuff */
+    void *client_destructor_data;
 };
 
 /* =Standard=interfaces=================================================== */
diff --git a/src/sbus/sssd_dbus_server.c b/src/sbus/sssd_dbus_server.c
index 900086924..50dfc680a 100644
--- a/src/sbus/sssd_dbus_server.c
+++ b/src/sbus/sssd_dbus_server.c
@@ -30,6 +30,11 @@
 
 static int sbus_server_destructor(void *ctx);
 
+struct new_connection_data {
+    struct sbus_connection *server;
+    void *client_destructor_data;
+};
+
 /*
  * new_connection_callback
  * Actions to be run upon each new client connection
@@ -41,19 +46,20 @@ static void sbus_server_init_new_connection(DBusServer *dbus_server,
                                             DBusConnection *dbus_conn,
                                             void *data)
 {
-    struct sbus_connection *server;
+    struct new_connection_data *ncd;
     struct sbus_connection *conn;
     int ret;
 
     DEBUG(SSSDBG_FUNC_DATA,"Entering.\n");
-    server = talloc_get_type(data, struct sbus_connection);
-    if (!server) {
+    ncd = talloc_get_type(data, struct new_connection_data);
+    if (!ncd) {
         return;
     }
 
     DEBUG(SSSDBG_FUNC_DATA,"Adding connection %p.\n", dbus_conn);
-    ret = sbus_init_connection(server, server->ev, dbus_conn,
-                               SBUS_CONN_TYPE_PRIVATE, NULL, &conn);
+    ret = sbus_init_connection(ncd->server, ncd->server->ev, dbus_conn,
+                               SBUS_CONN_TYPE_PRIVATE, NULL,
+                               ncd->client_destructor_data, &conn);
     if (ret != 0) {
         dbus_connection_close(dbus_conn);
         DEBUG(SSSDBG_FUNC_DATA, "Closing connection (failed setup)\n");
@@ -69,7 +75,7 @@ static void sbus_server_init_new_connection(DBusServer *dbus_server,
      * This function (or its callbacks) should also
      * set up connection-specific methods.
      */
-    ret = server->srv_init_fn(conn, server->srv_init_data);
+    ret = ncd->server->srv_init_fn(conn, ncd->server->srv_init_data);
     if (ret != EOK) {
         DEBUG(SSSDBG_CRIT_FAILURE,"Initialization failed!\n");
         dbus_connection_close(dbus_conn);
@@ -186,7 +192,8 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
                     bool use_symlink,
                     struct sbus_connection **_server,
                     sbus_server_conn_init_fn init_fn,
-                    void *init_pvt_data)
+                    void *init_pvt_data,
+                    void *client_destructor_data)
 {
     struct sbus_connection *server;
     DBusServer *dbus_server;
@@ -199,6 +206,7 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
     const char *socket_address;
     struct stat stat_buf;
     TALLOC_CTX *tmp_ctx;
+    struct new_connection_data *ncd;
 
     *_server = NULL;
 
@@ -309,10 +317,21 @@ int sbus_new_server(TALLOC_CTX *mem_ctx,
         }
     }
 
+    /* This structure must be alive while server is alive. That's the
+     * reason for using server as its talloc context.
+     */
+    ncd = talloc_zero((TALLOC_CTX *)server, struct new_connection_data);
+    if (!ncd) {
+        ret = ENOMEM;
+        goto done;
+    }
+    ncd->server = server;
+    ncd->client_destructor_data = client_destructor_data;
+
     /* Set up D-BUS new connection handler */
     dbus_server_set_new_connection_function(server->dbus.server,
                                             sbus_server_init_new_connection,
-                                            server, NULL);
+                                            ncd, NULL);
 
     /* Set up DBusWatch functions */
     dbret = dbus_server_set_watch_functions(server->dbus.server,
diff --git a/src/tests/common_dbus.c b/src/tests/common_dbus.c
index 1b0ae88dc..103512d7d 100644
--- a/src/tests/common_dbus.c
+++ b/src/tests/common_dbus.c
@@ -113,7 +113,8 @@ mock_server_child(void *data)
     loop = tevent_context_init(ctx);
 
     verify_eq (sbus_new_server(ctx, loop, mock->dbus_address, geteuid(), getegid(),
-                               false, &server, on_accept_connection, mock), EOK);
+                               false, &server, on_accept_connection, mock,
+                               NULL), EOK);
 
     tevent_add_fd(loop, ctx, mock->sync_fds[1], TEVENT_FD_READ,
                   on_sync_fd_written, &stop_server);