Skip to content

Commit

Permalink
GUACAMOLE-1846: Clean up rwlock code for public consumption.
Browse files Browse the repository at this point in the history
  • Loading branch information
jmuehlner committed Aug 30, 2023
1 parent 3b7495d commit 2769e5f
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 37 deletions.
8 changes: 4 additions & 4 deletions src/libguac/client.c
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ guac_client* guac_client_alloc() {
}

/* Init locks */
guac_init_rwlock(&(client->__users_lock));
guac_init_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);
Expand Down Expand Up @@ -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_rwlock(&(client->__users_lock));
guac_destroy_rwlock(&(client->__pending_users_lock));
guac_rwlock_destroy(&(client->__users_lock));
guac_rwlock_destroy(&(client->__pending_users_lock));

free(client->connection_id);
free(client);
Expand Down
2 changes: 1 addition & 1 deletion src/libguac/guacamole/client.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@
#include "client-types.h"
#include "client-constants.h"
#include "layer-types.h"
#include "guacamole/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"
Expand Down
39 changes: 17 additions & 22 deletions src/libguac/guacamole/rwlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_RWLOCK_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_rwlock_release_lock()
* before it can be reacquired.
*/
#define GUAC_RWLOCK_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,
Expand Down Expand Up @@ -82,15 +68,15 @@ typedef struct guac_rwlock {
* @param lock
* The guac reentrant rwlock to be initialized.
*/
void guac_init_rwlock(guac_rwlock* lock);
void guac_rwlock_init(guac_rwlock* lock);

/**
* Clean up and destroy the provided guac reentrant rwlock.
*
* @param lock
* The guac reentrant rwlock to be destroyed.
*/
void guac_destroy_rwlock(guac_rwlock* lock);
void guac_rwlock_destroy(guac_rwlock* lock);

/**
* Aquire the write lock for the provided guac reentrant rwlock, if the key does not
Expand All @@ -99,13 +85,16 @@ void guac_destroy_rwlock(guac_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_RWLOCK_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_rwlock_acquire_write_lock(guac_rwlock* reentrant_rwlock);

Expand All @@ -115,13 +104,16 @@ int guac_rwlock_acquire_write_lock(guac_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_RWLOCK_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_rwlock_acquire_read_lock(guac_rwlock* reentrant_rwlock);

Expand All @@ -131,12 +123,15 @@ int guac_rwlock_acquire_read_lock(guac_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_RWLOCK_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_rwlock_release_lock(guac_rwlock* reentrant_rwlock);

Expand Down
42 changes: 32 additions & 10 deletions src/libguac/rwlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

#include <pthread.h>
#include <stdint.h>
#include "guacamole/error.h"
#include "guacamole/rwlock.h"

/**
Expand All @@ -37,7 +38,7 @@
*/
#define GUAC_REENTRANT_LOCK_WRITE_LOCK 2

void guac_init_rwlock(guac_rwlock* lock) {
void guac_rwlock_init(guac_rwlock* lock) {

/* Configure to allow sharing this lock with child processes */
pthread_rwlockattr_t lock_attributes;
Expand All @@ -52,7 +53,7 @@ void guac_init_rwlock(guac_rwlock* lock) {

}

void guac_destroy_rwlock(guac_rwlock* lock) {
void guac_rwlock_destroy(guac_rwlock* lock) {

/* Destroy the rwlock */
pthread_rwlock_destroy(&(lock->lock));
Expand All @@ -68,7 +69,7 @@ void guac_destroy_rwlock(guac_rwlock* lock) {
* @param lock
* The guac reentrant rwlock to be destroyed.
*/
void guac_destroy_rwlock(guac_rwlock* lock);
void guac_rwlock_destroy(guac_rwlock* lock);

/**
* Extract and return the flag indicating which lock is held, if any, from the
Expand Down Expand Up @@ -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
Expand All @@ -152,8 +153,15 @@ int guac_rwlock_acquire_write_lock(guac_rwlock* reentrant_rwlock) {
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_RWLOCK_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) {
Expand Down Expand Up @@ -192,8 +200,15 @@ int guac_rwlock_acquire_read_lock(guac_rwlock* reentrant_rwlock) {
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_RWLOCK_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 (
Expand Down Expand Up @@ -230,8 +245,15 @@ int guac_rwlock_release_lock(guac_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_RWLOCK_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) {
Expand Down

0 comments on commit 2769e5f

Please sign in to comment.