Sulkta-OSS/element-x-ada
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Mitigate a deadlock when loading room timelines (#6674)

Browse source 
This may be happening because we were not destroying focused event timelines used for the media viewer/gallery when necessary, and having several of those back paginating *may* have caused a deadlock in the event cache.
This commit is contained in:
Jorge Martin Espinosa 2026-04-30 16:01:24 +02:00 • committed by GitHub
parent 13775f4fbd
commit 30fd90abb9
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 28 additions and 19 deletions
Download patch file Download diff file Expand all files Collapse all files

13
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/MediaGalleryDataSource.kt
View file

@ -17,6 +17,7 @@ import io.element.android.libraries.matrix.api.room.BaseRoom
import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.matrix.api.timeline.item.event.toEventOrTransactionId
import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
@ -27,10 +28,11 @@ import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onCompletion
import kotlinx.coroutines.flow.onEach
import timber.log.Timber
import java.util.concurrent.atomic.AtomicBoolean
interface MediaGalleryDataSource {
fun start()
fun start(coroutineScope: CoroutineScope)
fun groupedMediaItemsFlow(): Flow<AsyncData<GroupedMediaItems>>
fun getLastData(): AsyncData<GroupedMediaItems>
suspend fun loadMore(direction: Timeline.PaginationDirection)
@ -58,7 +60,7 @@ class TimelineMediaGalleryDataSource(
private val isStarted = AtomicBoolean(false)
@OptIn(ExperimentalCoroutinesApi::class)
override fun start() {
override fun start(coroutineScope: CoroutineScope) {
if (!isStarted.compareAndSet(false, true)) {
return
}
@ -96,9 +98,12 @@ class TimelineMediaGalleryDataSource(
groupedMediaItemsFlow.emit(AsyncData.Success(groupedMediaItems))
}
.onCompletion {
timeline?.close()
timeline?.let {
Timber.d("Timeline media gallery data source flow completed for room ${room.roomId}, closing timeline")
it.close()
}
}
.launchIn(room.roomCoroutineScope)
.launchIn(coroutineScope)
}
override suspend fun loadMore(direction: Timeline.PaginationDirection) {

2
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/gallery/MediaGalleryPresenter.kt
View file

@ -78,7 +78,7 @@ class MediaGalleryPresenter(
.collectAsState(AsyncData.Uninitialized)
LaunchedEffect(Unit) {
mediaGalleryDataSource.start()
mediaGalleryDataSource.start(this)
}
val permissions by room.permissionsAsState(MediaPermissions.DEFAULT) { perms ->

6
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSource.kt
View file

@ -35,6 +35,7 @@ import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toImmutableList
import kotlinx.coroutines.CoroutineDispatcher
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.withContext
@ -62,11 +63,12 @@ class MediaViewerDataSource(
private val localMediaStates: MutableMap<String, MutableState<AsyncData<LocalMedia>>> =
mutableMapOf()
fun setup() {
galleryDataSource.start()
fun setup(coroutineScope: CoroutineScope) {
galleryDataSource.start(coroutineScope)
}
fun dispose() {
Timber.d("Disposing MediaViewerDataSource, closing ${mediaFiles.size} media files")
mediaFiles.forEach { it.close() }
mediaFiles.clear()
localMediaStates.clear()

2
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerPresenter.kt
View file

@ -88,7 +88,7 @@ class MediaViewerPresenter(
var mediaBottomSheetState by remember { mutableStateOf<MediaBottomSheetState>(MediaBottomSheetState.Hidden) }
DisposableEffect(Unit) {
dataSource.setup()
dataSource.setup(coroutineScope)
onDispose {
dataSource.dispose()
}

3
libraries/mediaviewer/impl/src/main/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSource.kt
View file

@ -20,12 +20,13 @@ import io.element.android.libraries.mediaviewer.impl.datasource.MediaGalleryData
import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems
import io.element.android.libraries.mediaviewer.impl.model.MediaItem
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.flowOf
class SingleMediaGalleryDataSource(
private val data: GroupedMediaItems,
) : MediaGalleryDataSource {
override fun start() = Unit
override fun start(coroutineScope: CoroutineScope) = Unit
override fun groupedMediaItemsFlow() = flowOf(AsyncData.Success(data))
override fun getLastData(): AsyncData<GroupedMediaItems> = AsyncData.Success(data)

3
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/FakeMediaGalleryDataSource.kt
View file

@ -13,6 +13,7 @@ import io.element.android.libraries.matrix.api.core.EventId
import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.mediaviewer.impl.model.GroupedMediaItems
import io.element.android.tests.testutils.lambda.lambdaError
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
@ -21,7 +22,7 @@ class FakeMediaGalleryDataSource(
private val loadMoreLambda: (Timeline.PaginationDirection) -> Unit = { lambdaError() },
private val deleteItemLambda: (EventId) -> Unit = { lambdaError() },
) : MediaGalleryDataSource {
override fun start() = startLambda()
override fun start(coroutineScope: CoroutineScope) = startLambda()
private val groupedMediaItemsFlow = MutableSharedFlow<AsyncData<GroupedMediaItems>>(
replay = 1

12
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/datasource/TimelineMediaGalleryDataSourceTest.kt
View file

@ -80,7 +80,7 @@ class TimelineMediaGalleryDataSourceTest {
roomCoroutineScope = backgroundScope,
)
)
sut.start()
sut.start(backgroundScope)
assertThat(sut.getLastData()).isEqualTo(AsyncData.Uninitialized)
sut.groupedMediaItemsFlow().test {
assertThat(awaitItem().isLoading()).isTrue()
@ -95,7 +95,7 @@ class TimelineMediaGalleryDataSourceTest {
)
assertThat(sut.getLastData().isSuccess()).isTrue()
// Also test that starting again should have no effect
sut.start()
sut.start(backgroundScope)
}
}
// Ensure that the timeline has been closed on flow completion
@ -117,7 +117,7 @@ class TimelineMediaGalleryDataSourceTest {
roomCoroutineScope = backgroundScope,
)
)
sut.start()
sut.start(backgroundScope)
sut.groupedMediaItemsFlow().test {
skipItems(2)
sut.loadMore(Timeline.PaginationDirection.BACKWARDS)
@ -140,7 +140,7 @@ class TimelineMediaGalleryDataSourceTest {
roomCoroutineScope = backgroundScope,
)
)
sut.start()
sut.start(backgroundScope)
sut.groupedMediaItemsFlow().test {
skipItems(2)
sut.deleteItem(AN_EVENT_ID)
@ -159,7 +159,7 @@ class TimelineMediaGalleryDataSourceTest {
roomCoroutineScope = backgroundScope,
)
)
sut.start()
sut.start(backgroundScope)
sut.groupedMediaItemsFlow().test {
assertThat(awaitItem().isLoading()).isTrue()
assertThat(sut.getLastData().isLoading()).isTrue()
@ -181,7 +181,7 @@ class TimelineMediaGalleryDataSourceTest {
roomCoroutineScope = backgroundScope,
)
)
sut.start()
sut.start(backgroundScope)
sut.groupedMediaItemsFlow().test {
assertThat(awaitItem().isLoading()).isTrue()
assertThat(sut.getLastData().isLoading()).isTrue()

2
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/MediaViewerDataSourceTest.kt
View file

@ -50,7 +50,7 @@ class MediaViewerDataSourceTest {
val sut = createMediaViewerDataSource(
galleryDataSource = galleryDataSource,
)
sut.setup()
sut.setup(backgroundScope)
startLambda.assertions().isCalledOnce()
}

4
libraries/mediaviewer/impl/src/test/kotlin/io/element/android/libraries/mediaviewer/impl/viewer/SingleMediaGalleryDataSourceTest.kt
View file

@ -37,9 +37,9 @@ class SingleMediaGalleryDataSourceTest {
val warmUpRule = WarmUpRule()
@Test
fun `function start is no op`() {
fun `function start is no op`() = runTest {
val sut = SingleMediaGalleryDataSource(aGroupedMediaItems())
sut.start()
sut.start(backgroundScope)
}
@Test