From 7a65a63aa9c7e6721652b5f2b6c449bcf840ccb1 Mon Sep 17 00:00:00 2001
From: Michael Jumper <mjumper@apache.org>
Date: Fri, 7 Oct 2016 13:09:59 -0700
Subject: [PATCH] GUACAMOLE-171: Do not require knowledge of broadcast socket
 internals (do not acquire write lock around join/leave handlers).

---
 src/libguac/client.c           | 16 ++++++++--------
 src/libguac/guacamole/client.h | 18 ------------------
 2 files changed, 8 insertions(+), 26 deletions(-)

diff --git a/src/libguac/client.c b/src/libguac/client.c
index a3c37339..824254f6 100644
--- a/src/libguac/client.c
+++ b/src/libguac/client.c
@@ -273,12 +273,12 @@ int guac_client_add_user(guac_client* client, guac_user* user, int argc, char**
 
     int retval = 0;
 
-    pthread_rwlock_wrlock(&(client->__users_lock));
-
     /* Call handler, if defined */
     if (client->join_handler)
         retval = client->join_handler(user, argc, argv);
 
+    pthread_rwlock_wrlock(&(client->__users_lock));
+
     /* Add to list if join was successful */
     if (retval == 0) {
 
@@ -307,12 +307,6 @@ void guac_client_remove_user(guac_client* client, guac_user* user) {
 
     pthread_rwlock_wrlock(&(client->__users_lock));
 
-    /* Call handler, if defined */
-    if (user->leave_handler)
-        user->leave_handler(user);
-    else if (client->leave_handler)
-        client->leave_handler(user);
-
     /* Update prev / head */
     if (user->__prev != NULL)
         user->__prev->__next = user->__next;
@@ -331,6 +325,12 @@ void guac_client_remove_user(guac_client* client, guac_user* user) {
 
     pthread_rwlock_unlock(&(client->__users_lock));
 
+    /* Call handler, if defined */
+    if (user->leave_handler)
+        user->leave_handler(user);
+    else if (client->leave_handler)
+        client->leave_handler(user);
+
 }
 
 void guac_client_foreach_user(guac_client* client, guac_user_callback* callback, void* data) {
diff --git a/src/libguac/guacamole/client.h b/src/libguac/guacamole/client.h
index ccb49cf1..2d01573c 100644
--- a/src/libguac/guacamole/client.h
+++ b/src/libguac/guacamole/client.h
@@ -53,10 +53,6 @@ struct guac_client {
      * provide their own mechanism of I/O for their protocol. The guac_socket
      * structure is used only to communicate conveniently with the Guacamole
      * web-client.
-     *
-     * Because this socket broadcasts to all connected users, this socket MUST
-     * NOT be used within the same thread as a "leave" or "join" handler. Doing
-     * so results in undefined behavior, including possible segfaults.
      */
     guac_socket* socket;
 
@@ -437,10 +433,6 @@ void guac_client_remove_user(guac_client* client, guac_user* user);
  * MAY invoke guac_client_foreach_user(), doing so should not be necessary, and
  * may indicate poor design choices.
  *
- * Because this function loops through all connected users, this function MUST
- * NOT be invoked within the same thread as a "leave" or "join" handler. Doing
- * so results in undefined behavior, including possible segfaults.
- *
  * @param client
  *     The client whose users should be iterated.
  *
@@ -465,11 +457,6 @@ void guac_client_foreach_user(guac_client* client,
  * This function is reentrant, but the user list MUST NOT be manipulated
  * within the same thread as a callback to this function.
  *
- * Because this function depends on a consistent list of connected users, this
- * function MUST NOT be invoked within the same thread as a "leave" or "join"
- * handler. Doing so results in undefined behavior, including possible
- * segfaults.
- *
  * @param client
  *     The client to retrieve the owner from.
  *
@@ -498,11 +485,6 @@ void* guac_client_for_owner(guac_client* client, guac_user_callback* callback,
  * This function is reentrant, but the user list MUST NOT be manipulated
  * within the same thread as a callback to this function.
  *
- * Because this function depends on a consistent list of connected users, this
- * function MUST NOT be invoked within the same thread as a "leave" or "join"
- * handler. Doing so results in undefined behavior, including possible
- * segfaults.
- *
  * @param client
  *     The client that the given user is expected to be associated with.
  *