jmuehlner commented on code in PR #455:
URL: https://github.com/apache/guacamole-server/pull/455#discussion_r1308031814


##########
src/libguac/client.c:
##########
@@ -169,35 +290,56 @@ guac_client* guac_client_alloc() {
         client->__output_streams[i].index = GUAC_CLIENT_CLOSED_STREAM_INDEX;
     }
 
-
     /* Init locks */
-    pthread_rwlockattr_init(&lock_attributes);
-    pthread_rwlockattr_setpshared(&lock_attributes, PTHREAD_PROCESS_SHARED);
+    guac_init_reentrant_rwlock(&(client->__users_lock));
+    guac_init_reentrant_rwlock(&(client->__pending_users_lock));
 
-    pthread_rwlock_init(&(client->__users_lock), &lock_attributes);
+    /* Initialize the write lock flags to 0, as threads won't have yet */
+    pthread_key_create(&(client->__users_lock.key), (void *) 0);
+    pthread_key_create(&(client->__pending_users_lock.key), (void *) 0);
 
-    /* Set up socket to broadcast to all users */
+    /* 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);
 
     return client;
 
 }
 
 void guac_client_free(guac_client* client) {
 
+    /* Acquire write locks before referencing user pointers */

Review Comment:
   Added this extra locking layer here because it's not really valid to 
reference the `__users` or `__pending_users` fields without holding these 
locks. 
   
   I had thought about using `guac_client_remove_user` as a callback for 
`guac_client_foreach_user` and `guac_client_foreach_pending_user`, which is a 
more standard way of doing things, but that would also mean that'd be acquiring 
the read lock first, and upgrading to the write lock inside 
`guac_client_remove_user()`, which should technically work but should probably 
be avoided.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to