mike-jumper opened a new pull request, #405: URL: https://github.com/apache/guacamole-server/pull/405
These changes make a minor correction to the usage of `__users_lock` we were incorrectly re-acquiring the lock while it was already held for writing, and then double-releasing the lock. Overall flow is similar for both join/leave notification: 1. User joins/leaves connection and guacd enters `guac_client_add_user()` or `guac_client_remove_user()`. 2. The function in question acquires `__users_lock` for writing. 3. While the lock is held for writing, a function which calls `guac_client_for_owner()` is invoked. 4. `guac_client_for_owner()` acquires `__users_lock` for reading. The behavior of a thread doing this while it already holds the lock for writing is undefined according to POSIX (see below). 5. `guac_client_for_owner()` releases `__users_lock` (first unlock). 6. The function in question also releases `__users_lock` (second unlock). Per POSIX spec, the behavior of acquiring a read lock on a rwlock that's already acquired for writing is undefined. From the documentation for `pthread_rwlock_rdlock()`: > ... Results are undefined if the calling thread holds a write lock on rwlock at the time the call is made. Coverity caught this in part by noticing the effectively-double call to `pthread_rwlock_unlock()`, however it's also technically incorrect to re-acquire the lock while it's held in the first place. We can avoid both issues by simply invoking the relevant functions outside the scope of the lock. -- 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]
