Skip to content

Try to workaround a timeline pagination deadlock #2824

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

Closed
wants to merge 2 commits into from
Closed

Try to workaround a timeline pagination deadlock #2824

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7121a5e
Try to workaround a timeline pagination deadlock
jmartinesp May 9, 2024
7b73f17
Remove unnecessary `runBlocking` calls
jmartinesp May 9, 2024
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
2 changes: 1 addition & 1 deletion ...ue/impl/src/main/kotlin/io/element/android/features/ftue/impl/state/DefaultFtueService.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
44 changes: 26 additions & 18 deletions ...ix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/timeline/RustTimeline.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -101,6 +103,7 @@ class RustTimeline(
) : Timeline {
private val initLatch = CompletableDeferred<Unit>()
private val isInit = AtomicBoolean(false)
private val paginationMutex = Mutex()

private val _timelineItems: MutableStateFlow<List<MatrixTimelineItem>> =
MutableStateFlow(emptyList())
Expand Down Expand Up @@ -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<Boolean> = withContext(NonCancellable) {
override suspend fun paginate(direction: Timeline.PaginationDirection): Result<Boolean> = 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) }
}
}

Expand Down