diff --git a/src/guacd/daemon.c b/src/guacd/daemon.c index adfaa3c17..e867ada5a 100644 --- a/src/guacd/daemon.c +++ b/src/guacd/daemon.c @@ -497,8 +497,7 @@ int main(int argc, char* argv[]) { } /* Clean up and exit if SIGINT or SIGTERM signals are caught */ - struct sigaction signal_stop_action = { 0 }; - signal_stop_action.sa_handler = signal_stop_handler; + struct sigaction signal_stop_action = { .sa_handler = signal_stop_handler }; sigaction(SIGINT, &signal_stop_action, NULL); sigaction(SIGTERM, &signal_stop_action, NULL); diff --git a/src/guacd/proc.c b/src/guacd/proc.c index 936ad9085..9325575da 100644 --- a/src/guacd/proc.c +++ b/src/guacd/proc.c @@ -356,8 +356,7 @@ static void guacd_exec_proc(guacd_proc* proc, const char* protocol) { guacd_proc_self = proc; /* Clean up and exit if SIGINT or SIGTERM signals are caught */ - struct sigaction signal_stop_action = { 0 }; - signal_stop_action.sa_handler = signal_stop_handler; + struct sigaction signal_stop_action = { .sa_handler = signal_stop_handler }; sigaction(SIGINT, &signal_stop_action, NULL); sigaction(SIGTERM, &signal_stop_action, NULL); diff --git a/src/libguac/Makefile.am b/src/libguac/Makefile.am index 2097fe7ca..963644ed8 100644 --- a/src/libguac/Makefile.am +++ b/src/libguac/Makefile.am @@ -61,6 +61,7 @@ libguacinc_HEADERS = \ guacamole/protocol-constants.h \ guacamole/protocol-types.h \ guacamole/recording.h \ + guacamole/rwlock.h \ guacamole/socket-constants.h \ guacamole/socket.h \ guacamole/socket-fntypes.h \ @@ -82,7 +83,6 @@ noinst_HEADERS = \ id.h \ encode-jpeg.h \ encode-png.h \ - reentrant-rwlock.h \ palette.h \ user-handlers.h \ raw_encoder.h \ @@ -98,7 +98,7 @@ libguac_la_SOURCES = \ fips.c \ hash.c \ id.c \ - reentrant-rwlock.c \ + rwlock.c \ palette.c \ parser.c \ pool.c \ diff --git a/src/libguac/client.c b/src/libguac/client.c index ceb556ddc..04e6c9f81 100644 --- a/src/libguac/client.c +++ b/src/libguac/client.c @@ -28,13 +28,13 @@ #include "guacamole/plugin.h" #include "guacamole/pool.h" #include "guacamole/protocol.h" +#include "guacamole/rwlock.h" #include "guacamole/socket.h" #include "guacamole/stream.h" #include "guacamole/string.h" #include "guacamole/timestamp.h" #include "guacamole/user.h" #include "id.h" -#include "reentrant-rwlock.h" #include #include @@ -183,7 +183,7 @@ static void guac_client_promote_pending_users(union sigval data) { return; /* Acquire the lock for reading and modifying the list of pending users */ - guac_acquire_write_lock(&(client->__pending_users_lock)); + guac_rwlock_acquire_write_lock(&(client->__pending_users_lock)); /* Run the pending join handler, if one is defined */ if (client->join_pending_handler) { @@ -191,7 +191,7 @@ static void guac_client_promote_pending_users(union sigval data) { /* If an error occurs in the pending handler */ if(client->join_pending_handler(client)) { - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); /* Mark the handler as not running */ pthread_mutex_lock(&(client->__pending_users_timer_mutex)); @@ -223,7 +223,7 @@ static void guac_client_promote_pending_users(union sigval data) { client->__pending_users = NULL; /* Acquire the lock for reading and modifying the list of full users. */ - guac_acquire_write_lock(&(client->__users_lock)); + guac_rwlock_acquire_write_lock(&(client->__users_lock)); /* If any users were removed from the pending list, promote them now */ if (last_user != NULL) { @@ -237,11 +237,11 @@ static void guac_client_promote_pending_users(union sigval data) { } - guac_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); /* Release the lock (this is done AFTER updating the connected user list * to ensure that all users are always on exactly one of these lists) */ - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); /* Mark the handler as complete so the next instance can run */ pthread_mutex_lock(&(client->__pending_users_timer_mutex)); @@ -291,8 +291,8 @@ guac_client* guac_client_alloc() { } /* Init locks */ - guac_init_reentrant_rwlock(&(client->__users_lock)); - guac_init_reentrant_rwlock(&(client->__pending_users_lock)); + guac_rwlock_init(&(client->__users_lock)); + guac_rwlock_init(&(client->__pending_users_lock)); /* Initialize the write lock flags to 0, as threads won't have yet */ pthread_key_create(&(client->__users_lock.key), (void *) 0); @@ -315,8 +315,8 @@ guac_client* guac_client_alloc() { void guac_client_free(guac_client* client) { /* Acquire write locks before referencing user pointers */ - guac_acquire_write_lock(&(client->__pending_users_lock)); - guac_acquire_write_lock(&(client->__users_lock)); + guac_rwlock_acquire_write_lock(&(client->__pending_users_lock)); + guac_rwlock_acquire_write_lock(&(client->__users_lock)); /* Remove all pending users */ while (client->__pending_users != NULL) @@ -327,8 +327,8 @@ void guac_client_free(guac_client* client) { guac_client_remove_user(client, client->__users); /* Release the locks */ - guac_release_lock(&(client->__users_lock)); - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); if (client->free_handler) { @@ -371,8 +371,8 @@ void guac_client_free(guac_client* client) { 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)); + guac_rwlock_destroy(&(client->__users_lock)); + guac_rwlock_destroy(&(client->__pending_users_lock)); free(client->connection_id); free(client); @@ -451,7 +451,7 @@ static void guac_client_add_pending_user( guac_client* client, guac_user* user) { /* Acquire the lock for modifying the list of pending users */ - guac_acquire_write_lock(&(client->__pending_users_lock)); + guac_rwlock_acquire_write_lock(&(client->__pending_users_lock)); user->__prev = NULL; user->__next = client->__pending_users; @@ -465,7 +465,7 @@ static void guac_client_add_pending_user( client->connected_users++; /* Release the lock */ - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); } @@ -494,10 +494,10 @@ static int guac_client_start_pending_users_timer(guac_client* client) { } /* Configure the timer to synchronize and clear the pending users */ - struct sigevent signal_config = { 0 }; - signal_config.sigev_notify = SIGEV_THREAD; - signal_config.sigev_notify_function = guac_client_promote_pending_users; - signal_config.sigev_value.sival_ptr = client; + struct sigevent signal_config = { + .sigev_notify = SIGEV_THREAD, + .sigev_notify_function = guac_client_promote_pending_users, + .sigev_value = { .sival_ptr = client }}; /* Create a timer to synchronize any pending users periodically */ if (timer_create( @@ -509,9 +509,10 @@ static int guac_client_start_pending_users_timer(guac_client* client) { } /* Configure the pending users timer to run on the defined interval */ - struct itimerspec time_config = { 0 }; - time_config.it_interval.tv_nsec = GUAC_CLIENT_PENDING_USERS_REFRESH_INTERVAL; - time_config.it_value.tv_nsec = GUAC_CLIENT_PENDING_USERS_REFRESH_INTERVAL; + struct itimerspec time_config = { + .it_interval = { .tv_nsec = GUAC_CLIENT_PENDING_USERS_REFRESH_INTERVAL }, + .it_value = { .tv_nsec = GUAC_CLIENT_PENDING_USERS_REFRESH_INTERVAL } + }; /* Start the timer */ if (timer_settime( @@ -574,8 +575,8 @@ int guac_client_add_user(guac_client* client, guac_user* user, int argc, char** void guac_client_remove_user(guac_client* client, guac_user* user) { - guac_acquire_write_lock(&(client->__pending_users_lock)); - guac_acquire_write_lock(&(client->__users_lock)); + guac_rwlock_acquire_write_lock(&(client->__pending_users_lock)); + guac_rwlock_acquire_write_lock(&(client->__users_lock)); /* Update prev / head */ if (user->__prev != NULL) @@ -595,8 +596,8 @@ void guac_client_remove_user(guac_client* client, guac_user* user) { if (user->owner) client->__owner = NULL; - guac_release_lock(&(client->__users_lock)); - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); /* Update owner of user having left the connection. */ if (!user->owner) @@ -614,7 +615,7 @@ void guac_client_foreach_user(guac_client* client, guac_user_callback* callback, guac_user* current; - guac_acquire_read_lock(&(client->__users_lock)); + guac_rwlock_acquire_read_lock(&(client->__users_lock)); /* Call function on each user */ current = client->__users; @@ -623,7 +624,7 @@ void guac_client_foreach_user(guac_client* client, guac_user_callback* callback, current = current->__next; } - guac_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); } @@ -632,7 +633,7 @@ void guac_client_foreach_pending_user( guac_user* current; - guac_acquire_read_lock(&(client->__pending_users_lock)); + guac_rwlock_acquire_read_lock(&(client->__pending_users_lock)); /* Call function on each pending user */ current = client->__pending_users; @@ -641,7 +642,7 @@ void guac_client_foreach_pending_user( current = current->__next; } - guac_release_lock(&(client->__pending_users_lock)); + guac_rwlock_release_lock(&(client->__pending_users_lock)); } @@ -650,12 +651,12 @@ void* guac_client_for_owner(guac_client* client, guac_user_callback* callback, void* retval; - guac_acquire_read_lock(&(client->__users_lock)); + guac_rwlock_acquire_read_lock(&(client->__users_lock)); /* Invoke callback with current owner */ retval = callback(client->__owner, data); - guac_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); /* Return value from callback */ return retval; @@ -670,7 +671,7 @@ void* guac_client_for_user(guac_client* client, guac_user* user, int user_valid = 0; void* retval; - guac_acquire_read_lock(&(client->__users_lock)); + guac_rwlock_acquire_read_lock(&(client->__users_lock)); /* Loop through all users, searching for a pointer to the given user */ current = client->__users; @@ -692,7 +693,7 @@ void* guac_client_for_user(guac_client* client, guac_user* user, /* Invoke callback with requested user (if they exist) */ retval = callback(user, data); - guac_release_lock(&(client->__users_lock)); + guac_rwlock_release_lock(&(client->__users_lock)); /* Return value from callback */ return retval; diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h index 9b7868704..fbb89a727 100644 --- a/src/libguac/guacamole/client.h +++ b/src/libguac/guacamole/client.h @@ -30,9 +30,9 @@ #include "client-types.h" #include "client-constants.h" #include "layer-types.h" -#include "reentrant-rwlock.h" #include "object-types.h" #include "pool-types.h" +#include "rwlock.h" #include "socket-types.h" #include "stream-types.h" #include "timestamp-types.h" @@ -172,7 +172,7 @@ struct guac_client { * Lock which is acquired when the users list is being manipulated, or when * the users list is being iterated. */ - guac_reentrant_rwlock __users_lock; + guac_rwlock __users_lock; /** * The first user within the list of all connected users, or NULL if no @@ -184,7 +184,7 @@ struct guac_client { * Lock which is acquired when the pending users list is being manipulated, * or when the pending users list is being iterated. */ - guac_reentrant_rwlock __pending_users_lock; + guac_rwlock __pending_users_lock; /** * A timer that will periodically synchronize the list of pending users, diff --git a/src/libguac/reentrant-rwlock.h b/src/libguac/guacamole/rwlock.h similarity index 73% rename from src/libguac/reentrant-rwlock.h rename to src/libguac/guacamole/rwlock.h index 92bc5b6a0..719a8e303 100644 --- a/src/libguac/reentrant-rwlock.h +++ b/src/libguac/guacamole/rwlock.h @@ -17,8 +17,8 @@ * under the License. */ -#ifndef __GUAC_REENTRANT_LOCK_H -#define __GUAC_REENTRANT_LOCK_H +#ifndef __GUAC_RWLOCK_H +#define __GUAC_RWLOCK_H #include @@ -38,20 +38,6 @@ * unexpected behavior. */ -/** - * An error code indicating that the calling thread is attempting to release a - * lock that it does not control. - */ -#define GUAC_REEANTRANT_LOCK_ERROR_DOUBLE_RELEASE 1 - -/** - * The lock cannot be acquired because the lock has been already been - * reentrantly acquired too many times, exhausting the capacity of this library - * to track this lock. The lock must be released using guac_release_lock() - * before it can be reacquired. - */ -#define GUAC_REEANTRANT_LOCK_ERROR_TOO_MANY 2 - /** * A structure packaging together a pthread rwlock along with a key to a * thread-local property to keep track of the current status of the lock, @@ -59,7 +45,7 @@ * Note that both the lock and key must be initialized before being provided * to any of these functions. */ -typedef struct guac_reentrant_rwlock { +typedef struct guac_rwlock { /** * A non-reentrant pthread rwlock to be wrapped by the local lock, @@ -73,7 +59,7 @@ typedef struct guac_reentrant_rwlock { */ pthread_key_t key; -} guac_reentrant_rwlock; +} guac_rwlock; /** * Initialize the provided guac reentrant rwlock. The lock will be configured to be @@ -82,7 +68,7 @@ typedef struct guac_reentrant_rwlock { * @param lock * The guac reentrant rwlock to be initialized. */ -void guac_init_reentrant_rwlock(guac_reentrant_rwlock* lock); +void guac_rwlock_init(guac_rwlock* lock); /** * Clean up and destroy the provided guac reentrant rwlock. @@ -90,7 +76,7 @@ void guac_init_reentrant_rwlock(guac_reentrant_rwlock* lock); * @param lock * The guac reentrant rwlock to be destroyed. */ -void guac_destroy_reentrant_rwlock(guac_reentrant_rwlock* lock); +void guac_rwlock_destroy(guac_rwlock* lock); /** * Aquire the write lock for the provided guac reentrant rwlock, if the key does not @@ -99,15 +85,18 @@ void guac_destroy_reentrant_rwlock(guac_reentrant_rwlock* lock); * write lock is acquired. The thread local property associated with the key * will be updated as necessary to track the thread's ownership of the lock. * + * If an error occurs while attempting to acquire the lock, a non-zero value is + * returned, and guac_error is set appropriately. + * * @param reentrant_rwlock * The guac reentrant rwlock for which the write lock should be acquired * reentrantly. * * @return - * Zero if the lock is succesfully acquired, or an error code defined above - * by a GUAC_REEANTRANT_LOCK_ERROR_* constant if the lock cannot be acquired. + * Zero if the lock is succesfully acquired, or a non-zero value if an + * error occured. */ -int guac_acquire_write_lock(guac_reentrant_rwlock* reentrant_rwlock); +int guac_rwlock_acquire_write_lock(guac_rwlock* reentrant_rwlock); /** * Aquire the read lock for the provided guac reentrant rwlock, if the key does not @@ -115,15 +104,18 @@ int guac_acquire_write_lock(guac_reentrant_rwlock* reentrant_rwlock); * property associated with the key will be updated as necessary to track the * thread's ownership of the lock. * + * If an error occurs while attempting to acquire the lock, a non-zero value is + * returned, and guac_error is set appropriately. + * * @param reentrant_rwlock * The guac reentrant rwlock for which the read lock should be acquired * reentrantly. * * @return - * Zero if the lock is succesfully acquired, or an error code defined above - * by a GUAC_REEANTRANT_LOCK_ERROR_* constant if the lock cannot be acquired. + * Zero if the lock is succesfully acquired, or a non-zero value if an + * error occured. */ -int guac_acquire_read_lock(guac_reentrant_rwlock* reentrant_rwlock); +int guac_rwlock_acquire_read_lock(guac_rwlock* reentrant_rwlock); /** * Release the the rwlock associated with the provided guac reentrant rwlock if this @@ -131,14 +123,17 @@ int guac_acquire_read_lock(guac_reentrant_rwlock* reentrant_rwlock); * local property associated with the key will be updated as needed to ensure * that the correct number of release requests will finally release the lock. * + * If an error occurs while attempting to release the lock, a non-zero value is + * returned, and guac_error is set appropriately. + * * @param reentrant_rwlock * The guac reentrant rwlock that should be released. * * @return - * Zero if the lock is succesfully released, or an error code defined above - * by a GUAC_REEANTRANT_LOCK_ERROR_* constant if the lock cannot be released. + * Zero if the lock is succesfully released, or a non-zero value if an + * error occured. */ -int guac_release_lock(guac_reentrant_rwlock* reentrant_rwlock); +int guac_rwlock_release_lock(guac_rwlock* reentrant_rwlock); #endif diff --git a/src/libguac/reentrant-rwlock.c b/src/libguac/rwlock.c similarity index 86% rename from src/libguac/reentrant-rwlock.c rename to src/libguac/rwlock.c index 1ac550128..e400c8e72 100644 --- a/src/libguac/reentrant-rwlock.c +++ b/src/libguac/rwlock.c @@ -19,7 +19,8 @@ #include #include -#include "reentrant-rwlock.h" +#include "guacamole/error.h" +#include "guacamole/rwlock.h" /** * The value indicating that the current thread holds neither the read or write @@ -37,7 +38,7 @@ */ #define GUAC_REENTRANT_LOCK_WRITE_LOCK 2 -void guac_init_reentrant_rwlock(guac_reentrant_rwlock* lock) { +void guac_rwlock_init(guac_rwlock* lock) { /* Configure to allow sharing this lock with child processes */ pthread_rwlockattr_t lock_attributes; @@ -52,7 +53,7 @@ void guac_init_reentrant_rwlock(guac_reentrant_rwlock* lock) { } -void guac_destroy_reentrant_rwlock(guac_reentrant_rwlock* lock) { +void guac_rwlock_destroy(guac_rwlock* lock) { /* Destroy the rwlock */ pthread_rwlock_destroy(&(lock->lock)); @@ -68,7 +69,7 @@ void guac_destroy_reentrant_rwlock(guac_reentrant_rwlock* lock) { * @param lock * The guac reentrant rwlock to be destroyed. */ -void guac_destroy_reentrant_rwlock(guac_reentrant_rwlock* lock); +void guac_rwlock_destroy(guac_rwlock* lock); /** * Extract and return the flag indicating which lock is held, if any, from the @@ -125,7 +126,7 @@ static void* get_value_from_flag_and_count( /** * Return zero if adding one to the current count would overflow the storage - * allocated to the count, or a non-zero value otherwise. + * allocated to the count, or a non-zero value otherwise. * * @param current_count * The current count for a lock that the current thread is trying to @@ -145,15 +146,22 @@ static int would_overflow_count(uintptr_t current_count) { } -int guac_acquire_write_lock(guac_reentrant_rwlock* reentrant_rwlock) { +int guac_rwlock_acquire_write_lock(guac_rwlock* reentrant_rwlock) { uintptr_t key_value = (uintptr_t) pthread_getspecific(reentrant_rwlock->key); uintptr_t flag = get_lock_flag(key_value); uintptr_t count = get_lock_count(key_value); /* If acquiring this lock again would overflow the counter storage */ - if (would_overflow_count(count)) - return GUAC_REEANTRANT_LOCK_ERROR_TOO_MANY; + if (would_overflow_count(count)) { + + guac_error = GUAC_STATUS_TOO_MANY; + guac_error_message = "Unable to acquire write lock because there's" + " insufficient space to store another level of lock depth"; + + return 1; + + } /* If the current thread already holds the write lock, increment the count */ if (flag == GUAC_REENTRANT_LOCK_WRITE_LOCK) { @@ -185,15 +193,22 @@ int guac_acquire_write_lock(guac_reentrant_rwlock* reentrant_rwlock) { } -int guac_acquire_read_lock(guac_reentrant_rwlock* reentrant_rwlock) { +int guac_rwlock_acquire_read_lock(guac_rwlock* reentrant_rwlock) { uintptr_t key_value = (uintptr_t) pthread_getspecific(reentrant_rwlock->key); uintptr_t flag = get_lock_flag(key_value); uintptr_t count = get_lock_count(key_value); /* If acquiring this lock again would overflow the counter storage */ - if (would_overflow_count(count)) - return GUAC_REEANTRANT_LOCK_ERROR_TOO_MANY; + if (would_overflow_count(count)) { + + guac_error = GUAC_STATUS_TOO_MANY; + guac_error_message = "Unable to acquire read lock because there's" + " insufficient space to store another level of lock depth"; + + return 1; + + } /* The current thread may read if either the read or write lock is held */ if ( @@ -220,7 +235,7 @@ int guac_acquire_read_lock(guac_reentrant_rwlock* reentrant_rwlock) { } -int guac_release_lock(guac_reentrant_rwlock* reentrant_rwlock) { +int guac_rwlock_release_lock(guac_rwlock* reentrant_rwlock) { uintptr_t key_value = (uintptr_t) pthread_getspecific(reentrant_rwlock->key); uintptr_t flag = get_lock_flag(key_value); @@ -230,8 +245,15 @@ int guac_release_lock(guac_reentrant_rwlock* reentrant_rwlock) { * Return an error if an attempt is made to release a lock that the current * thread does not control. */ - if (count <= 0) - return GUAC_REEANTRANT_LOCK_ERROR_DOUBLE_RELEASE; + if (count <= 0) { + + guac_error = GUAC_STATUS_INVALID_ARGUMENT; + guac_error_message = "Unable to free rwlock because it's not held by" + " the current thread"; + + return 1; + + } /* Release the lock if this is the last locked level */ if (count == 1) {