uid for each connected client#96
Merged
Merged
Conversation
Theldus
approved these changes
Sep 5, 2024
Owner
Theldus
left a comment
Theldus
left a comment
There was a problem hiding this comment.
Hi @henriquetft,
Your PR is exactly how I envisioned a UID implementation, thank you very much for your work on this.
Since you said you will also update the README.md and man-pages, I will wait before merging this, but it is already approved =).
Contributor
Author
|
I've just pushed another commit to update the README and manpages. Let me know if anything else needs adjustment! |
This was referenced Sep 5, 2024
Theldus
added a commit
that referenced
this pull request
Sep 16, 2024
The bug was introduced in PR #96, which added support for a client UID. The issue is that by adding a "UID," it also necessitates looping through the list of clients to find the connection corresponding to the specific UID. This loop needs to be protected by a mutex to prevent modifications to the client list while it's being traversed, and this part works perfectly fine. The problem arises when a ping broadcast is sent to all active clients. In that case, the client list is also locked with the same mutex, causing the code to enter a blocking state. The fix is quite simple: switch from using `ws_sendframe()` to `ws_sendframe_internal()`, since the latter does not rely on the client's UID but instead uses the connection structure, which was already obtained earlier. This way, no additional locks are needed, and the code runs without issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #66
Please check if the implementation matches what you had in mind. If it does, I'll proceed and update the README and the man pages.