Sign out crash - Realm configuration mismatch #4480
Conversation
… always querying the session manager - fixes the close sesion flow causing the session to be recreated
…read which holds the handler lazily provides itself meaning the session exists and is in sync
| @@ -174,8 +174,8 @@ internal class DefaultSession @Inject constructor( | |||
| lifecycleObservers.forEach { | |||
| it.onSessionStarted(this) | |||
| } | |||
| sessionListeners.dispatch { _, listener -> | |||
| listener.onSessionStarted(this) | |||
There was a problem hiding this comment.
as we were already avoiding the session parameter in the callback it felt like the SessionListeners didn't actually need to hold the session itself
this also means that the close flow can safely use the session instance without worrying about it dissapearing from the SessionManager which would cause onSessionStopped to not be called
| @@ -50,7 +51,7 @@ private const val GET_GROUP_DATA_WORKER = "GET_GROUP_DATA_WORKER" | |||
|
|
|||
| internal class SyncResponseHandler @Inject constructor( | |||
| @SessionDatabase private val monarchy: Monarchy, | |||
| @SessionId private val sessionId: String, | |||
| private val session: Session, | |||
There was a problem hiding this comment.
I wasn't sure about this change, technically this works as the SyncThread uses a Dagger Provider so the instance is always correct but it feels a little overkill~
There was a problem hiding this comment.
Yes, we should not inject the Session in the SDK (I have checked and this is never done, AFAICS)
Is is required to update this file to fix the issue?
There was a problem hiding this comment.
we need a session instance in order to call
sessionListeners.dispatch(session) { session, listener ->
listener.onNewInvitedRoom(session, roomId)
}I could pass the SessionManager to the SyncResponseHandler like the SessionListeners was previously doing
There was a problem hiding this comment.
405cda7 it's an inlined version of what the SessionListeners was doing
There was a problem hiding this comment.
I think we could also replace onNewInvitedRoom by listening to the Flow instead
There was a problem hiding this comment.
@ganfra yes, please do that in another PR :p
| @@ -42,18 +38,19 @@ internal class SessionListeners @Inject constructor( | |||
| } | |||
| } | |||
|
|
|||
| fun dispatch(block: (Session, Session.Listener) -> Unit) { | |||
| fun dispatch(session: Session, block: (Session, Session.Listener) -> Unit) { | |||
There was a problem hiding this comment.
this is the fix, instead of relying on sessionManager.getSessionComponent(sessionId)?.session() which may create a new session if one doesn't exist, the callers provide a known session instance
|
This black icon is strange, maybe due to the video recording/compression?
|
I'm not actually sure, I thought it was meant to be like this 😅 going frame by frame I can see it transition from white to black
|
Fixes an intermittent crash on sign out caused by a race condition where we attempt to access an already closed session causing it to be recreated which in turn causes realm to crash because the new session is slightly different.
the race condition boils down to...
sessionComponents.remove(sessionId) runOnUiThread { if (sessionParamsStore[sessionId] != null) { val session = sessionComponents.getOrCreate(sessionId).session() onStopped(session) } } sessionParamsStore.delete(sessionId)SessionManagerwithin theSessionListenersand instead pass the current session directly (to avoid creating a new one), which we were already doing in most cases