Fix to get websocketInstance #2852
Merged
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
|
/backport to stable-16.0 |
mahibi
force-pushed
the
bugfix/noid/fixWebsocketConnection
branch
from
7d7af29 to
baa7c56
Compare
AndyScherzinger
approved these changes
Mar 9, 2023
mahibi
added
2. developing
Add toast warning for debug mode Rename "magic" stuff Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
This is necessary as many of the workers store user data. When running in parallel, there are race conditions and user data that was stored by one worker gets directly overwritten by the next worker. E.g. this happened with the "externalSignalingServer" attribute of user: SignalingSettingsWorker saved the user with the externalSignalingServer value, but then CapabilitiesWorker kicked in and saved the user without this value. Because of this, in WebsocketConnectionsWorker getExternalSignalingServer() of the user was null. Because of this, webSocketConnectionHelper.getExternalSignalingInstanceForServer(..) was not called. So the webSocketInstanceMap in WebSocketConnectionHelper was never filled. This is why WebSocketConnectionHelper.getMagicWebSocketInstanceForUserId(..) in ChatController failed to get a webSocketInstance. Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
this is already done in onAttach which seems to do the job fine after using workManager queues and fixing to set externalSignalingServer to user. See commits da1714bb and 29a37086 Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
mahibi
force-pushed
the
bugfix/noid/fixWebsocketConnection
branch
from
baa7c56 to
abd1d4b
Compare
mahibi
commented
Mar 10, 2023
| // Ensure that the listener is added if the web socket instance was not set up yet when | ||
| // onAttach() was called. | ||
| webSocketInstance?.getSignalingMessageReceiver()?.addListener(localParticipantMessageListener) | ||
|
|
There was a problem hiding this comment.
fyi @danxuliu this is now solved...
mahibi
added
3. to review
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
AlvaroBrey
approved these changes
Mar 10, 2023
app/src/main/java/com/nextcloud/talk/webrtc/WebSocketConnectionHelper.java
Show resolved
Hide resolved
app/src/main/java/com/nextcloud/talk/application/NextcloudTalkApplication.kt
Show resolved
Hide resolved
AndyScherzinger
approved these changes
Mar 10, 2023
Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
|
APK file: https://www.kaminsky.me/nc-dev/android-artifacts/2852-talk.apk |
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.
This PR fixes to set the externalSignalingServer to the user, see 8c991c6
Furthermore, race conditions of Workers are now avoided by using WorkManager chains, see 609e5a2
🏁 Checklist
/backport to stable-xx.x