From ac1889bfdcb77622bbc818352d65634209d9829f Mon Sep 17 00:00:00 2001 From: =?utf8?q?J=C3=A9r=C3=A9mie=20Galarneau?= Date: Fri, 7 Feb 2020 23:21:40 -0500 Subject: [PATCH] sessiond: notification: maintain an id to notification_client ht MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Change-Id: I7b8774c23a4a2a7c41ed8c1652f23d29954a0771 --- .../notification-thread-events.c | 80 +++++++++++++++---- src/bin/lttng-sessiond/notification-thread.c | 10 +++ src/bin/lttng-sessiond/notification-thread.h | 20 ++++- 3 files changed, 92 insertions(+), 18 deletions(-) diff --git a/src/bin/lttng-sessiond/notification-thread-events.c b/src/bin/lttng-sessiond/notification-thread-events.c index 6f0ee7d79..51b95ee6d 100644 --- 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 7dadb2c67..c964e7c91 100644 --- 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 3d766780e..21cc086c6 100644 --- a/src/bin/lttng-sessiond/notification-thread.h +++ b/src/bin/lttng-sessiond/notification-thread.h @@ -19,6 +19,9 @@ #include #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. */ -- 2.34.1