From b9d46fbadb263252cc9c39821cedc4589f742b9d Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Wed, 23 Aug 2023 23:54:01 +0000 Subject: [PATCH] GUACAMOLE-1846: Migrate away from unsupported atomic state for pending user promotion. --- src/libguac/client.c | 72 +++++++++++++++++++++++++++------- src/libguac/guacamole/client.h | 14 ++----- 2 files changed, 61 insertions(+), 25 deletions(-) diff --git a/src/libguac/client.c b/src/libguac/client.c index 46a0118cfc..2822cde4cc 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -42,7 +42,6 @@ #include #include #include -#include #include #include #include @@ -53,6 +52,24 @@ */ #define GUAC_CLIENT_PENDING_USERS_REFRESH_INTERVAL 250000000 +/** + * A value that indicates that the pending users timer has yet to be + * initialized and started. + */ +#define GUAC_CLIENT_PENDING_TIMER_UNREGISTERED 0 + +/** + * A value that indicates that the pending users timer has been initialized + * and started, but that the timer handler is not currently running. + */ +#define GUAC_CLIENT_PENDING_TIMER_REGISTERED 1 + +/** + * A value that indicates that the pending users timer has been initialized + * and started, and that the timer handler is currently running. + */ +#define GUAC_CLIENT_PENDING_TIMER_TRIGGERED 2 + /** * Empty NULL-terminated array of argument names. */ @@ -149,8 +166,20 @@ static void guac_client_promote_pending_users(union sigval data) { guac_client* client = (guac_client*) data.sival_ptr; - /* Do not start if the previous promotion event is still running */ - if (atomic_flag_test_and_set(&(client->__pending_timer_event_active))) + pthread_mutex_lock(&(client->__pending_users_timer_mutex)); + + /* Check if the previous instance of this handler is still running */ + int already_running = ( + client->__pending_users_timer_state + == GUAC_CLIENT_PENDING_TIMER_TRIGGERED); + + /* Mark the handler as running if it isn't already */ + client->__pending_users_timer_state = GUAC_CLIENT_PENDING_TIMER_TRIGGERED; + + pthread_mutex_unlock(&(client->__pending_users_timer_mutex)); + + /* Do not start the handler if the previous instance is still running */ + if (already_running) return; /* Acquire the lock for reading and modifying the list of pending users */ @@ -197,8 +226,10 @@ static void guac_client_promote_pending_users(union sigval data) { * to ensure that all users are always on exactly one of these lists) */ guac_release_lock(&(client->__pending_users_lock)); - /* Mark the timer event as complete so the next instance can run */ - atomic_flag_clear(&(client->__pending_timer_event_active)); + /* Mark the handler as complete so the next instance can run */ + pthread_mutex_lock(&(client->__pending_users_timer_mutex)); + client->__pending_users_timer_state = GUAC_CLIENT_PENDING_TIMER_REGISTERED; + pthread_mutex_unlock(&(client->__pending_users_timer_mutex)); } @@ -250,16 +281,16 @@ guac_client* guac_client_alloc() { pthread_key_create(&(client->__users_lock.key), (void *) 0); pthread_key_create(&(client->__pending_users_lock.key), (void *) 0); - /* Ensure the timer is constructed only once */ + /* The timer will be lazily created in the child process */ + client->__pending_users_timer_state = GUAC_CLIENT_PENDING_TIMER_UNREGISTERED; + + /* Set up the pending user promotion mutex */ pthread_mutex_init(&(client->__pending_users_timer_mutex), NULL); /* Set up broadcast sockets */ client->socket = guac_socket_broadcast(client); client->pending_socket = guac_socket_broadcast_pending(client); - /* Set the timer event thread as initially inactive, since it hasn't run */ - atomic_flag_clear(&(client->__pending_timer_event_active)); - return client; } @@ -301,12 +332,20 @@ void guac_client_free(guac_client* client) { guac_client_log(client, GUAC_LOG_ERROR, "Unable to close plugin: %s", dlerror()); } - /* Destroy the pending users timer */ - pthread_mutex_destroy(&(client->__pending_users_timer_mutex)); - if (client->__pending_users_timer_running != 0) + /* Find out if the pending user promotion timer was ever started */ + pthread_mutex_lock(&(client->__pending_users_timer_mutex)); + int was_started = ( + client->__pending_users_timer_state + != GUAC_CLIENT_PENDING_TIMER_UNREGISTERED); + pthread_mutex_unlock(&(client->__pending_users_timer_mutex)); + + /* If the timer was registered, stop it before destroying the lock */ + if (was_started) timer_delete(client->__pending_users_timer); - /* Destroy the reenrant read-write locks */ + pthread_mutex_destroy(&(client->__pending_users_timer_mutex)); + + /* Destroy the reentrant read-write locks */ guac_destroy_reentrant_rwlock(&(client->__users_lock)); guac_destroy_reentrant_rwlock(&(client->__pending_users_lock)); @@ -423,7 +462,8 @@ static int guac_client_start_pending_users_timer(guac_client* client) { pthread_mutex_lock(&(client->__pending_users_timer_mutex)); /* Return success if the timer is already created and running */ - if (client->__pending_users_timer_running != 0) { + if (client->__pending_users_timer_state + != GUAC_CLIENT_PENDING_TIMER_UNREGISTERED) { pthread_mutex_unlock(&(client->__pending_users_timer_mutex)); return 0; } @@ -456,7 +496,9 @@ static int guac_client_start_pending_users_timer(guac_client* client) { return 1; } - client->__pending_users_timer_running = 1; + /* Mark the timer as registered but not yet running */ + client->__pending_users_timer_state = GUAC_CLIENT_PENDING_TIMER_REGISTERED; + pthread_mutex_unlock(&(client->__pending_users_timer_mutex)); return 0; diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h index a8552cb9e1..3ba806f5e6 100644 --- a/src/libguac/guacamole/client.h +++ b/src/libguac/guacamole/client.h @@ -196,22 +196,16 @@ struct guac_client { timer_t __pending_users_timer; /** - * Non-zero if the pending users timer is configured and running, or zero - * otherwise. + * A flag storing the current state of the pending users timer. */ - int __pending_users_timer_running; + int __pending_users_timer_state; /** - * A mutex that must be acquired before modifying the pending users timer. + * A mutex that must be acquired before modifying or checking the value of + * the timer state. */ pthread_mutex_t __pending_users_timer_mutex; - /** - * A flag that indicates whether the pending users timer event thread is - * currently running. - */ - volatile atomic_flag __pending_timer_event_active; - /** * The first user within the list of connected users who have not yet had * their connection states synchronized after joining.