From 7121a5e600a2955f90183fccf916d8357edaab5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 9 May 2024 11:34:15 +0200 Subject: [PATCH 1/2] Try to workaround a timeline pagination deadlock Added a Mutex to avoid several pagination runs at the same time. This *seems* to workaround the issue. --- .../matrix/impl/timeline/RustTimeline.kt | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt index 3ec21e7560e..d253c1e65e9 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt @@ -50,10 +50,10 @@ import io.element.android.libraries.matrix.impl.timeline.postprocessor.LoadingIn import io.element.android.libraries.matrix.impl.timeline.postprocessor.RoomBeginningPostProcessor import io.element.android.libraries.matrix.impl.timeline.postprocessor.TimelineEncryptedHistoryPostProcessor import io.element.android.services.toolbox.api.systemclock.SystemClock +import kotlinx.coroutines.CancellationException import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.ensureActive import kotlinx.coroutines.flow.Flow @@ -66,6 +66,8 @@ import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext import org.matrix.rustcomponents.sdk.EventTimelineItem import org.matrix.rustcomponents.sdk.FormattedBody @@ -101,6 +103,7 @@ class RustTimeline( ) : Timeline { private val initLatch = CompletableDeferred() private val isInit = AtomicBoolean(false) + private val paginationMutex = Mutex() private val _timelineItems: MutableStateFlow> = MutableStateFlow(emptyList()) @@ -172,25 +175,30 @@ class RustTimeline( } } - // Use NonCancellable to avoid breaking the timeline when the coroutine is cancelled. - override suspend fun paginate(direction: Timeline.PaginationDirection): Result = withContext(NonCancellable) { + override suspend fun paginate(direction: Timeline.PaginationDirection): Result = withContext(dispatcher) { initLatch.await() - runCatching { - if (!canPaginate(direction)) throw TimelineException.CannotPaginate - updatePaginationStatus(direction) { it.copy(isPaginating = true) } - when (direction) { - Timeline.PaginationDirection.BACKWARDS -> inner.paginateBackwards(PAGINATION_SIZE.toUShort()) - Timeline.PaginationDirection.FORWARDS -> inner.focusedPaginateForwards(PAGINATION_SIZE.toUShort()) - } - }.onFailure { error -> - updatePaginationStatus(direction) { it.copy(isPaginating = false) } - if (error is TimelineException.CannotPaginate) { - Timber.d("Can't paginate $direction on room ${matrixRoom.roomId} with paginationStatus: ${backPaginationStatus.value}") - } else { - Timber.e(error, "Error paginating $direction on room ${matrixRoom.roomId}") + // Performing 2 paginations at the same time sometimes leads to a deadlock + paginationMutex.withLock { + runCatching { + if (!canPaginate(direction)) throw TimelineException.CannotPaginate + updatePaginationStatus(direction) { it.copy(isPaginating = true) } + when (direction) { + Timeline.PaginationDirection.BACKWARDS -> inner.paginateBackwards(PAGINATION_SIZE.toUShort()) + Timeline.PaginationDirection.FORWARDS -> inner.focusedPaginateForwards(PAGINATION_SIZE.toUShort()) + } + }.onFailure { error -> + if (error is CancellationException) { + throw error + } + updatePaginationStatus(direction) { it.copy(isPaginating = false) } + if (error is TimelineException.CannotPaginate) { + Timber.d("Can't paginate $direction on room ${matrixRoom.roomId} with paginationStatus: ${backPaginationStatus.value}") + } else { + Timber.e(error, "Error paginating $direction on room ${matrixRoom.roomId}") + } + }.onSuccess { hasReachedEnd -> + updatePaginationStatus(direction) { it.copy(isPaginating = false, hasMoreToLoad = !hasReachedEnd) } } - }.onSuccess { hasReachedEnd -> - updatePaginationStatus(direction) { it.copy(isPaginating = false, hasMoreToLoad = !hasReachedEnd) } } } From 7b73f17593f280df68f432053a13bd44494f41f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jorge=20Mart=C3=ADn?= Date: Thu, 9 May 2024 11:34:39 +0200 Subject: [PATCH 2/2] Remove unnecessary `runBlocking` calls --- .../android/features/ftue/impl/state/DefaultFtueService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt index 0e8aa54f819..14fbe2b9646 100644 --- a/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt +++ b/features/ftue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt @@ -131,7 +131,7 @@ class DefaultFtueService @Inject constructor( private suspend fun shouldAskNotificationPermissions(): Boolean { return if (sdkVersionProvider.isAtLeast(Build.VERSION_CODES.TIRAMISU)) { val permission = Manifest.permission.POST_NOTIFICATIONS - val isPermissionDenied = runBlocking { permissionStateProvider.isPermissionDenied(permission).first() } + val isPermissionDenied = permissionStateProvider.isPermissionDenied(permission).first() val isPermissionGranted = permissionStateProvider.isPermissionGranted(permission) !isPermissionGranted && !isPermissionDenied } else {