GUACAMOLE-1732: Merge deadlock and cleanup fixes for RDP client.

src/protocols/rdp/client.c

@@ -35,10 +35,12 @@
 #include "common-ssh/user.h"
 #endif
 
+#include <guacamole/argv.h>
 #include <guacamole/audio.h>
 #include <guacamole/client.h>
 #include <guacamole/mem.h>
 #include <guacamole/recording.h>
+#include <guacamole/rwlock.h>
 
 #include <dirent.h>
 #include <errno.h>
@@ -117,7 +119,7 @@ static int guac_rdp_join_pending_handler(guac_client* client) {
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
     guac_socket* broadcast_socket = client->pending_socket;
 
-    pthread_rwlock_rdlock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
 
     /* Synchronize any audio stream for each pending user */
     if (rdp_client->audio)
@@ -133,7 +135,7 @@ static int guac_rdp_join_pending_handler(guac_client* client) {
         guac_socket_flush(broadcast_socket);
     }
 
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     return 0;
 
@@ -220,7 +222,7 @@ int guac_client_init(guac_client* client, int argc, char** argv) {
             PTHREAD_MUTEX_RECURSIVE);
 
     /* Init required locks */
-    pthread_rwlock_init(&(rdp_client->lock), NULL);
+    guac_rwlock_init(&(rdp_client->lock));
     pthread_mutex_init(&(rdp_client->message_lock), &(rdp_client->attributes));
 
     /* Set handlers */
@@ -241,6 +243,14 @@ int guac_rdp_client_free_handler(guac_client* client) {
 
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
+    /*
+     * Signals any threads that are blocked awaiting user input for authentication
+     * (e.g., username or password) to terminate their wait. By broadcasting a
+     * condition signal, the authentication process is interrupted, allowing for
+     * premature termination and cleanup during client disconnection.
+     */
+    guac_argv_stop();
+
     /* Wait for client thread */
     pthread_join(rdp_client->client_thread, NULL);
 
@@ -297,7 +307,7 @@ int guac_rdp_client_free_handler(guac_client* client) {
     if (rdp_client->audio_input != NULL)
         guac_rdp_audio_buffer_free(rdp_client->audio_input);
 
-    pthread_rwlock_destroy(&(rdp_client->lock));
+    guac_rwlock_destroy(&(rdp_client->lock));
     pthread_mutex_destroy(&(rdp_client->message_lock));
 
     /* Free client data */
@@ -306,4 +316,3 @@ int guac_rdp_client_free_handler(guac_client* client) {
     return 0;
 
 }
-

src/protocols/rdp/input.c

@@ -30,6 +30,7 @@
 #include <freerdp/input.h>
 #include <guacamole/client.h>
 #include <guacamole/recording.h>
+#include <guacamole/rwlock.h>
 #include <guacamole/user.h>
 
 #include <stdlib.h>
@@ -39,7 +40,7 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {
     guac_client* client = user->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    pthread_rwlock_rdlock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
 
     /* Skip if not yet connected */
     freerdp* rdp_inst = rdp_client->rdp_inst;
@@ -125,7 +126,7 @@ int guac_rdp_user_mouse_handler(guac_user* user, int x, int y, int mask) {
     }
 
 complete:
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     return 0;
 }
@@ -136,7 +137,7 @@ int guac_rdp_user_touch_handler(guac_user* user, int id, int x, int y,
     guac_client* client = user->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    pthread_rwlock_rdlock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
 
     /* Skip if not yet connected */
     freerdp* rdp_inst = rdp_client->rdp_inst;
@@ -152,7 +153,7 @@ int guac_rdp_user_touch_handler(guac_user* user, int id, int x, int y,
     guac_rdp_rdpei_touch_update(rdp_client->rdpei, id, x, y, force);
 
 complete:
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     return 0;
 }
@@ -163,7 +164,7 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
     int retval = 0;
 
-    pthread_rwlock_rdlock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
 
     /* Report key state within recording */
     if (rdp_client->recording != NULL)
@@ -179,7 +180,7 @@ int guac_rdp_user_key_handler(guac_user* user, int keysym, int pressed) {
                 keysym, pressed, GUAC_RDP_KEY_SOURCE_CLIENT);
 
 complete:
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     return retval;
 
@@ -202,4 +203,3 @@ int guac_rdp_user_size_handler(guac_user* user, int width, int height) {
     return 0;
 
 }
-

src/protocols/rdp/keyboard.c

@@ -26,6 +26,7 @@
 #include <freerdp/input.h>
 #include <guacamole/client.h>
 #include <guacamole/mem.h>
+#include <guacamole/rwlock.h>
 
 #include <stdlib.h>
 
@@ -718,7 +719,7 @@ BOOL guac_rdp_keyboard_set_indicators(rdpContext* context, UINT16 flags) {
     guac_client* client = ((rdp_freerdp_context*) context)->client;
     guac_rdp_client* rdp_client = (guac_rdp_client*) client->data;
 
-    pthread_rwlock_rdlock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
 
     /* Skip if keyboard not yet ready */
     guac_rdp_keyboard* keyboard = rdp_client->keyboard;
@@ -730,7 +731,7 @@ BOOL guac_rdp_keyboard_set_indicators(rdpContext* context, UINT16 flags) {
     keyboard->lock_flags = flags;
 
 complete:
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
     return TRUE;
 
 }

src/protocols/rdp/rdp.c

@@ -108,8 +108,14 @@ BOOL rdp_freerdp_pre_connect(freerdp* instance) {
 
     /* Load "AUDIO_INPUT" plugin for audio input*/
     if (settings->enable_audio_input) {
+        /* Upgrade the lock to write temporarily for setting the newly allocated audio buffer */
+        guac_rwlock_acquire_write_lock(&(rdp_client->lock));
         rdp_client->audio_input = guac_rdp_audio_buffer_alloc(client);
         guac_rdp_audio_load_plugin(instance->context);
+
+        /* Downgrade the lock to allow for concurrent read access */
+        guac_rwlock_release_lock(&(rdp_client->lock));
+        guac_rwlock_acquire_read_lock(&(rdp_client->lock));
     }
 
     /* Load "cliprdr" service if not disabled */
@@ -470,7 +476,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
     /* Init random number generator */
     srandom(time(NULL));
 
-    pthread_rwlock_wrlock(&(rdp_client->lock));
+    guac_rwlock_acquire_write_lock(&(rdp_client->lock));
 
     /* Create display */
     rdp_client->display = guac_common_display_alloc(client,
@@ -516,12 +522,23 @@ static int guac_rdp_handle_connection(guac_client* client) {
     /* Set default pointer */
     guac_common_cursor_set_pointer(rdp_client->display->cursor);
 
+    /* 
+     * Downgrade the lock to allow for concurrent read access.
+     * Access to read locks needs to be made available for other processes such
+     * as the join_pending_handler to use while we await credentials from the user.
+     */
+    guac_rwlock_release_lock(&(rdp_client->lock));
+    guac_rwlock_acquire_read_lock(&(rdp_client->lock));
+
     /* Connect to RDP server */
     if (!freerdp_connect(rdp_inst)) {
         guac_rdp_client_abort(client, rdp_inst);
         goto fail;
     }
 
+    /* Upgrade to write lock again for further exclusive operations */
+    guac_rwlock_acquire_write_lock(&(rdp_client->lock));
+
     /* Connection complete */
     rdp_client->rdp_inst = rdp_inst;
 
@@ -530,7 +547,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
     /* Signal that reconnect has been completed */
     guac_rdp_disp_reconnect_complete(rdp_client->disp);
 
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     /* Handle messages from RDP server while client is running */
     while (client->state == GUAC_CLIENT_RUNNING
@@ -617,7 +634,7 @@ static int guac_rdp_handle_connection(guac_client* client) {
 
     }
 
-    pthread_rwlock_wrlock(&(rdp_client->lock));
+    guac_rwlock_acquire_write_lock(&(rdp_client->lock));
 
     /* Clean up print job, if active */
     if (rdp_client->active_job != NULL) {
@@ -652,15 +669,15 @@ static int guac_rdp_handle_connection(guac_client* client) {
     guac_common_display_free(rdp_client->display);
     rdp_client->display = NULL;
 
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
 
     /* Client is now disconnected */
     guac_client_log(client, GUAC_LOG_INFO, "Internal RDP client disconnected");
 
     return 0;
 
 fail:
-    pthread_rwlock_unlock(&(rdp_client->lock));
+    guac_rwlock_release_lock(&(rdp_client->lock));
     return 1;
 
 }

src/protocols/rdp/rdp.h

@@ -44,6 +44,7 @@
 #include <freerdp/freerdp.h>
 #include <guacamole/audio.h>
 #include <guacamole/client.h>
+#include <guacamole/rwlock.h>
 #include <guacamole/recording.h>
 #include <winpr/wtypes.h>
 
@@ -170,7 +171,7 @@ typedef struct guac_rdp_client {
      * from running when RDP data structures are allocated or freed
      * by the client thread.
      */
-    pthread_rwlock_t lock;
+    guac_rwlock lock;
 
     /**
      * Lock which synchronizes the sending of each RDP message, ensuring
@@ -219,4 +220,3 @@ typedef struct rdp_freerdp_context {
 void* guac_rdp_client_thread(void* data);
 
 #endif
-