sessiond: notification: maintain an id to notification_client ht

In preparation for the addition of an action execution worker, add a
client_id_ht which will allow the action worker to send commands to
the notification thread referencing clients by 'id' rather than by
their socket.

This is done in order to prevent FD re-use races between the various
threads at play when a communication error occurs on a notification
client socket.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I7b8774c23a4a2a7c41ed8c1652f23d29954a0771

diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c
index 6f0ee7d7942cf762b38c6c7edc37ab97caa8972b..51b95ee6da2e85b2501eda2b839cd39629ff760c 100644 (file)
--- a/src/bin/lttng-sessiond/notification-thread-events.c
+++ b/src/bin/lttng-sessiond/notification-thread-events.c
@@ -131,6 +131,7 @@ struct notification_client_list {
 };
 
 struct notification_client {
+       notification_client_id id;
        int socket;
        /* Client protocol version. */
        uint8_t major, minor;
@@ -148,6 +149,7 @@ struct notification_client {
         */
        struct cds_list_head condition_list;
        struct cds_lfht_node client_socket_ht_node;
+       struct cds_lfht_node client_id_ht_node;
        struct {
                struct {
                        /*
@@ -262,16 +264,25 @@ int lttng_session_trigger_list_add(struct lttng_session_trigger_list *list,
 
 
 static
-int match_client(struct cds_lfht_node *node, const void *key)
+int match_client_socket(struct cds_lfht_node *node, const void *key)
 {
        /* This double-cast is intended to supress pointer-to-cast warning. */
-       int socket = (int) (intptr_t) key;
-       struct notification_client *client;
+       const int socket = (int) (intptr_t) key;
+       const struct notification_client *client = caa_container_of(node,
+                       struct notification_client, client_socket_ht_node);
 
-       client = caa_container_of(node, struct notification_client,
-                       client_socket_ht_node);
+       return client->socket == socket;
+}
+
+static
+int match_client_id(struct cds_lfht_node *node, const void *key)
+{
+       /* This double-cast is intended to supress pointer-to-cast warning. */
+       const notification_client_id id = *((notification_client_id *) key);
+       const struct notification_client *client = caa_container_of(
+                       node, struct notification_client, client_id_ht_node);
 
-       return !!(client->socket == socket);
+       return client->id == id;
 }
 
 static
@@ -475,6 +486,18 @@ unsigned long hash_channel_key(struct channel_key *key)
        return key_hash ^ domain_hash;
 }
 
+static
+unsigned long hash_client_socket(int socket)
+{
+       return hash_key_ulong((void *) (unsigned long) socket, lttng_ht_seed);
+}
+
+static
+unsigned long hash_client_id(notification_client_id id)
+{
+       return hash_key_u64(&id, lttng_ht_seed);
+}
+
 /*
  * Get the type of object to which a given condition applies. Bindings let
  * the notification system evaluate a trigger's condition when a given
@@ -1138,6 +1161,7 @@ void notification_client_destroy(struct notification_client *client,
 
        if (client->socket >= 0) {
                (void) lttcomm_close_unix_sock(client->socket);
+               client->socket = -1;
        }
        lttng_dynamic_buffer_reset(&client->communication.inbound.buffer);
        lttng_dynamic_buffer_reset(&client->communication.outbound.buffer);
@@ -1157,8 +1181,8 @@ struct notification_client *get_client_from_socket(int socket,
        struct notification_client *client = NULL;
 
        cds_lfht_lookup(state->client_socket_ht,
-                       hash_key_ulong((void *) (unsigned long) socket, lttng_ht_seed),
-                       match_client,
+                       hash_client_socket(socket),
+                       match_client_socket,
                        (void *) (unsigned long) socket,
                        &iter);
        node = cds_lfht_iter_get_node(&iter);
@@ -1172,6 +1196,34 @@ end:
        return client;
 }
 
+/*
+ * Call with rcu_read_lock held (and hold for the lifetime of the returned
+ * client pointer).
+ */
+static
+struct notification_client *get_client_from_id(notification_client_id id,
+               struct notification_thread_state *state)
+{
+       struct cds_lfht_iter iter;
+       struct cds_lfht_node *node;
+       struct notification_client *client = NULL;
+
+       cds_lfht_lookup(state->client_id_ht,
+                       hash_client_id(id),
+                       match_client_id,
+                       &id,
+                       &iter);
+       node = cds_lfht_iter_get_node(&iter);
+       if (!node) {
+               goto end;
+       }
+
+       client = caa_container_of(node, struct notification_client,
+                       client_id_ht_node);
+end:
+       return client;
+}
+
 static
 bool buffer_usage_condition_applies_to_channel(
                const struct lttng_condition *condition,
@@ -2347,12 +2399,6 @@ error:
        return -1;
 }
 
-static
-unsigned long hash_client_socket(int socket)
-{
-       return hash_key_ulong((void *) (unsigned long) socket, lttng_ht_seed);
-}
-
 static
 int socket_set_non_blocking(int socket)
 {
@@ -2411,6 +2457,7 @@ int handle_notification_thread_client_connect(
                ret = -1;
                goto error;
        }
+       client->id = state->next_notification_client_id++;
        CDS_INIT_LIST_HEAD(&client->condition_list);
        lttng_dynamic_buffer_init(&client->communication.inbound.buffer);
        lttng_dynamic_buffer_init(&client->communication.outbound.buffer);
@@ -2459,6 +2506,9 @@ int handle_notification_thread_client_connect(
        cds_lfht_add(state->client_socket_ht,
                        hash_client_socket(client->socket),
                        &client->client_socket_ht_node);
+       cds_lfht_add(state->client_id_ht,
+                       hash_client_id(client->id),
+                       &client->client_id_ht_node);
        rcu_read_unlock();
 
        return ret;
@@ -2492,6 +2542,8 @@ int handle_notification_thread_client_disconnect(
        }
        cds_lfht_del(state->client_socket_ht,
                        &client->client_socket_ht_node);
+        cds_lfht_del(state->client_id_ht,
+                       &client->client_id_ht_node);
        notification_client_destroy(client, state);
 end:
        rcu_read_unlock();

diff --git a/src/bin/lttng-sessiond/notification-thread.c b/src/bin/lttng-sessiond/notification-thread.c
index 7dadb2c6748a7ae6738dad9294144407996a4d0d..c964e7c910b062a82753c50c58b78d728a9454c1 100644 (file)
--- a/src/bin/lttng-sessiond/notification-thread.c
+++ b/src/bin/lttng-sessiond/notification-thread.c
@@ -332,6 +332,10 @@ void fini_thread_state(struct notification_thread_state *state)
                ret = cds_lfht_destroy(state->client_socket_ht, NULL);
                assert(!ret);
        }
+       if (state->client_id_ht) {
+               ret = cds_lfht_destroy(state->client_id_ht, NULL);
+               assert(!ret);
+       }
        if (state->triggers_ht) {
                ret = handle_notification_thread_trigger_unregister_all(state);
                assert(!ret);
@@ -424,6 +428,12 @@ int init_thread_state(struct notification_thread_handle *handle,
                goto error;
        }
 
+       state->client_id_ht = cds_lfht_new(DEFAULT_HT_SIZE, 1, 0,
+                       CDS_LFHT_AUTO_RESIZE | CDS_LFHT_ACCOUNTING, NULL);
+       if (!state->client_id_ht) {
+               goto error;
+       }
+
        state->channel_triggers_ht = cds_lfht_new(DEFAULT_HT_SIZE, 1, 0,
                        CDS_LFHT_AUTO_RESIZE | CDS_LFHT_ACCOUNTING, NULL);
        if (!state->channel_triggers_ht) {

diff --git a/src/bin/lttng-sessiond/notification-thread.h b/src/bin/lttng-sessiond/notification-thread.h
index 3d766780e6fcc8a35e54cf53553cd7fdf6d19e60..21cc086c6ee8ec9c2db4b7b9c0daceee28ab1b99 100644 (file)
--- a/src/bin/lttng-sessiond/notification-thread.h
+++ b/src/bin/lttng-sessiond/notification-thread.h
@@ -19,6 +19,9 @@
 #include <semaphore.h>
 #include "thread.h"
 
+
+typedef uint64_t notification_client_id;
+
 struct notification_thread_handle {
        /*
         * Queue of struct notification command.
@@ -49,9 +52,14 @@ struct notification_thread_handle {
  * In order to speed-up and simplify queries, hash tables providing the
  * following associations are maintained:
  *
- *   - client_socket_ht: associate a client's socket (fd) to its "struct client"
- *             This hash table owns the "struct client" which must thus be
- *             disposed-of on removal from the hash table.
+ *   - client_socket_ht: associate a client's socket (fd) to its
+ *             "struct notification_client".
+ *             This hash table owns the "struct notification_client" which must
+ *             thus be disposed-of on removal from the hash table.
+ *
+ *   - client_id_ht: associate a client's id to its "struct notification_client"
+ *             This hash table holds a _weak_ reference to the
+ *             "struct notification_client".
  *
  *   - channel_triggers_ht:
  *             associates a channel key to a list of
@@ -168,9 +176,11 @@ struct notification_thread_handle {
  * 7) Session rotation completed
  *
  * 8) Connection of a client
- *    - add client socket to the client_socket_ht.
+ *    - add client socket to the client_socket_ht,
+ *    - add client socket to the client_id_ht.
  *
  * 9) Disconnection of a client
+ *    - remove client socket from the client_id_ht,
  *    - remove client socket from the client_socket_ht,
  *    - traverse all conditions to which the client is subscribed and remove
  *      the client from the notification_trigger_clients_ht.
@@ -191,6 +201,7 @@ struct notification_thread_state {
        int notification_channel_socket;
        struct lttng_poll_event events;
        struct cds_lfht *client_socket_ht;
+       struct cds_lfht *client_id_ht;
        struct cds_lfht *channel_triggers_ht;
        struct cds_lfht *session_triggers_ht;
        struct cds_lfht *channel_state_ht;
@@ -198,6 +209,7 @@ struct notification_thread_state {
        struct cds_lfht *channels_ht;
        struct cds_lfht *sessions_ht;
        struct cds_lfht *triggers_ht;
+       notification_client_id next_notification_client_id;
 };
 
 /* notification_thread_data takes ownership of the channel monitor pipes. */