Fix current user not included in user list returned by signaling #1522
Conversation
This file contains 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
When a user requests the signaling messages from the internal signaling server first the last ping of the user is updated. Then, after waiting for at most 30 seconds, the list of users active in the room is returned. That list was based on the users whose last ping was more recent than 30 seconds ago, so when there were no other messages and the waiting timed out the last ping of the current user set when the request started happened 30 seconds ago or more, and thus the current user was not included in the returned list (unless her ping was updated to a more recent value by a different request, like polling for chat messages). Now the users returned in the list are those whose last ping is more recent than or equal to 30 seconds, or more recent than or equal to the signaling ping of the current user if it is larger than 30 seconds. Signed-off-by: Daniel Calviño Sánchez <[email protected]>
danxuliu
added
3. to review
bug
feature: signaling 📶
|
/backport to stable15 |
Ivansss
deleted the
fix-current-user-not-included-in-user-list-returned-by-signaling
branch
|
backport to stable15 in #1523 |
This was referenced Feb 8, 2019
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
When a user requests the signaling messages from the internal signaling server first the last ping of the user is updated. Then, after waiting for at most 30 seconds, the list of users active in the room is returned.
That list was based on the users whose last ping was more recent than 30 seconds ago, so when there were no other messages and the waiting timed out the last ping of the current user set when the request started happened 30 seconds ago or more, and thus the current user was not included in the returned list... unles her ping was updated to a more recent value by a different request.
Requesting the chat messages also updates the last ping of a user, which usually masked this issue. However, when both the signaling and the chat controller updated the last ping at the same second and the wait in the signaling timed out the current user was not returned in the list. In the web UI, when the current user is not in the user list returned by the signaling the user is disconnected from the others.
Due to all this sometimes, when the internal signaling server was used, a user was disconnected from a call for no apparent reason (although in real case scenarios it was eventually reconnected, which does not happen in the forced test scenario below).
Now the users returned in the list are those whose last ping is more recent than or equal to 30 seconds, or more recent than or equal to the signaling ping of the current user if it is larger than 30 seconds.
How to test:
ping()inChatController), so the returned list depends only on the signaling ping and the bug can be reproduced consistentlyResult with this pull request:
The call goes on without interruptions.
Result without this pull request:
After 30 seconds the users are disconnected; further waiting does not cause a reconnection, it is needed that one of the participants leaves the call and joins again (in which case the problem will happen again after 30 seconds).