Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sign out crash - Realm configuration mismatch #4480

Merged
merged 4 commits into from
Nov 17, 2021
Merged

Sign out crash - Realm configuration mismatch #4480

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
470d766
dispatching session events to a specified session instance instead of…
ouchadam Nov 16, 2021
708889e
injecting the session directly into the response handler, the sync th…
ouchadam Nov 16, 2021
6d3bad9
adding changelog entry
ouchadam Nov 16, 2021
405cda7
reverting the session injection change, delegating to the session man…
ouchadam Nov 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4480.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes intermittent crash on sign out due to the session being incorrectly recreated whilst being closed
16 changes: 8 additions & 8 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/DefaultSession.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ internal class DefaultSession @Inject constructor(
lifecycleObservers.forEach {
it.onSessionStarted(this)
}
sessionListeners.dispatch { _, listener ->
listener.onSessionStarted(this)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

dispatchTo(sessionListeners) { session, listener ->
listener.onSessionStarted(session)
}
}
}
Expand Down Expand Up @@ -217,8 +217,8 @@ internal class DefaultSession @Inject constructor(
// timelineEventDecryptor.destroy()
uiHandler.post {
lifecycleObservers.forEach { it.onSessionStopped(this) }
sessionListeners.dispatch { _, listener ->
listener.onSessionStopped(this)
dispatchTo(sessionListeners) { session, listener ->
listener.onSessionStopped(session)
}
}
cryptoService.get().close()
Expand Down Expand Up @@ -249,8 +249,8 @@ internal class DefaultSession @Inject constructor(
lifecycleObservers.forEach {
it.onClearCache(this)
}
sessionListeners.dispatch { _, listener ->
listener.onClearCache(this)
dispatchTo(sessionListeners) { session, listener ->
listener.onClearCache(session)
}
}
withContext(NonCancellable) {
Expand All @@ -260,8 +260,8 @@ internal class DefaultSession @Inject constructor(
}

override fun onGlobalError(globalError: GlobalError) {
sessionListeners.dispatch { _, listener ->
listener.onGlobalError(this, globalError)
dispatchTo(sessionListeners) { session, listener ->
listener.onGlobalError(session, globalError)
}
}

Expand Down
19 changes: 8 additions & 11 deletions matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/SessionListeners.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,11 @@ package org.matrix.android.sdk.internal.session

import org.matrix.android.sdk.api.extensions.tryOrNull
import org.matrix.android.sdk.api.session.Session
import org.matrix.android.sdk.internal.SessionManager
import org.matrix.android.sdk.internal.di.SessionId
import timber.log.Timber
import javax.inject.Inject

@SessionScope
internal class SessionListeners @Inject constructor(
@SessionId private val sessionId: String,
private val sessionManager: SessionManager) {
internal class SessionListeners @Inject constructor() {

private val listeners = mutableSetOf<Session.Listener>()

Expand All @@ -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) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

synchronized(listeners) {
val session = getSession() ?: return Unit.also {
Timber.w("You don't have any attached session")
}
listeners.forEach {
tryOrNull { block(session, it) }
}
}
}
}

private fun getSession(): Session? {
return sessionManager.getSessionComponent(sessionId)?.session()
internal fun Session?.dispatchTo(sessionListeners: SessionListeners, block: (Session, Session.Listener) -> Unit) {
if (this == null) {
Timber.w("You don't have any attached session")
return
}
sessionListeners.dispatch(this, block)
}
6 changes: 5 additions & 1 deletion ...android/src/main/java/org/matrix/android/sdk/internal/session/sync/SyncResponseHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@ import org.matrix.android.sdk.api.session.initsync.InitSyncStep
import org.matrix.android.sdk.api.session.sync.model.GroupsSyncResponse
import org.matrix.android.sdk.api.session.sync.model.RoomsSyncResponse
import org.matrix.android.sdk.api.session.sync.model.SyncResponse
import org.matrix.android.sdk.internal.SessionManager
import org.matrix.android.sdk.internal.crypto.DefaultCryptoService
import org.matrix.android.sdk.internal.di.SessionDatabase
import org.matrix.android.sdk.internal.di.SessionId
import org.matrix.android.sdk.internal.di.WorkManagerProvider
import org.matrix.android.sdk.internal.session.SessionListeners
import org.matrix.android.sdk.internal.session.dispatchTo
import org.matrix.android.sdk.internal.session.group.GetGroupDataWorker
import org.matrix.android.sdk.internal.session.initsync.ProgressReporter
import org.matrix.android.sdk.internal.session.initsync.reportSubtask
Expand All @@ -51,6 +53,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 sessionManager: SessionManager,
private val sessionListeners: SessionListeners,
private val workManagerProvider: WorkManagerProvider,
private val roomSyncHandler: RoomSyncHandler,
Expand Down Expand Up @@ -158,8 +161,9 @@ internal class SyncResponseHandler @Inject constructor(
}

private fun dispatchInvitedRoom(roomsSyncResponse: RoomsSyncResponse) {
val session = sessionManager.getSessionComponent(sessionId)?.session()
roomsSyncResponse.invite.keys.forEach { roomId ->
sessionListeners.dispatch { session, listener ->
session.dispatchTo(sessionListeners) { session, listener ->
listener.onNewInvitedRoom(session, roomId)
}
}
Expand Down