Close the threaded timelines created by ShareLocationPresenter and DefaultMediaSender

Both created a dedicated timeline for each operation performed in a thread, and
never closed it, leaking SDK resources. Apply the same treatment as PollRepository:
replace getTimeline() with a scoped withTimeline() helper, which closes the created
timeline once the operation completes, whether it succeeds, fails or gets cancelled.

In DefaultMediaSender.sendGallery(), waiting for the upload handler is now done
inside the block, so that the timeline stays open until the upload completes.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Benoit Marty
2026-09-14 15:21:14 +02:00
parent e2fb2962f6
commit ed250320b3
4 changed files with 132 additions and 27 deletions
@@ -217,8 +217,8 @@ class ShareLocationPresenter(
val replyMode = messageComposerContext.composerMode as? MessageComposerMode.Reply val replyMode = messageComposerContext.composerMode as? MessageComposerMode.Reply
val inReplyToEventId = replyMode?.eventId val inReplyToEventId = replyMode?.eventId
val geoUri = event.location.toGeoUri() val geoUri = event.location.toGeoUri()
getTimeline().flatMap { withTimeline { timeline ->
it.sendLocation( timeline.sendLocation(
body = generateBody(geoUri), body = generateBody(geoUri),
geoUri = geoUri, geoUri = geoUri,
description = null, description = null,
@@ -237,10 +237,22 @@ class ShareLocationPresenter(
) )
} }
private suspend fun getTimeline(): Result<Timeline> { /**
* Invokes [block] with the [Timeline] matching [timelineMode].
*
* For a thread, a dedicated timeline is created and closed once [block] returns, since such a
* timeline owns SDK resources. For the other modes, the live timeline is used, and is not
* closed here since it is owned by its room.
*/
private suspend fun <T> withTimeline(block: suspend (Timeline) -> Result<T>): Result<T> {
return when (timelineMode) { return when (timelineMode) {
is Timeline.Mode.Thread -> room.createTimeline(CreateTimelineParams.Threaded(timelineMode.threadRootId)) is Timeline.Mode.Thread -> {
else -> Result.success(room.liveTimeline) room.createTimeline(CreateTimelineParams.Threaded(timelineMode.threadRootId))
.flatMap { threadedTimeline ->
threadedTimeline.use { block(it) }
}
}
else -> block(room.liveTimeline)
} }
} }
} }
@@ -708,6 +708,45 @@ class ShareLocationPresenterTest {
} }
} }
@Test
fun `ShareStaticLocation in a thread closes the created timeline`() = runTest {
val threadedTimeline = FakeTimeline(mode = Timeline.Mode.Thread(A_THREAD_ID)).apply {
sendLocationLambda = { _, _, _, _, _, _ -> Result.success(Unit) }
}
val liveTimeline = FakeTimeline()
val joinedRoom = FakeJoinedRoom(
liveTimeline = liveTimeline,
createTimelineResult = { Result.success(threadedTimeline) },
)
val shareLocationPresenter = createShareLocationPresenter(
joinedRoom = joinedRoom,
timelineMode = Timeline.Mode.Thread(A_THREAD_ID),
)
fakePermissionsPresenter.givenState(
aPermissionsState(
permissions = PermissionsState.Permissions.AllGranted,
shouldShowRationale = false,
)
)
shareLocationPresenter.test {
val initialState = awaitFirstItem()
initialState.eventSink(
ShareLocationEvent.ShareStaticLocation(
location = Location(lat = 3.0, lon = 4.0, accuracy = 5.0f),
isPinned = false,
)
)
advanceUntilIdle()
assertThat(threadedTimeline.closeCounter).isEqualTo(1)
assertThat(liveTimeline.closeCounter).isEqualTo(0)
cancelAndIgnoreRemainingEvents()
}
}
private suspend fun <T> ReceiveTurbine<T>.awaitFirstItem(): T { private suspend fun <T> ReceiveTurbine<T>.awaitFirstItem(): T {
skipItems(1) skipItems(1)
return awaitItem() return awaitItem()
@@ -100,9 +100,9 @@ class DefaultMediaSender(
inReplyToEventId: EventId?, inReplyToEventId: EventId?,
): Result<Unit> { ): Result<Unit> {
val mediaLogId = mediaId(mediaUploadInfo.file) val mediaLogId = mediaId(mediaUploadInfo.file)
return getTimeline().flatMap { return withTimeline { timeline ->
Timber.d("Started sending media $mediaLogId using timeline: ${it.mode}") Timber.d("Started sending media $mediaLogId using timeline: ${timeline.mode}")
it.sendMedia( timeline.sendMedia(
uploadInfo = mediaUploadInfo, uploadInfo = mediaUploadInfo,
caption = caption, caption = caption,
formattedCaption = formattedCaption, formattedCaption = formattedCaption,
@@ -128,12 +128,14 @@ class DefaultMediaSender(
mediaOptimizationConfig = mediaOptimizationConfig, mediaOptimizationConfig = mediaOptimizationConfig,
) )
.flatMapCatching { info -> .flatMapCatching { info ->
getTimeline().getOrThrow().sendMedia( withTimeline { timeline ->
uploadInfo = info, timeline.sendMedia(
caption = caption, uploadInfo = info,
formattedCaption = formattedCaption, caption = caption,
inReplyToEventId = inReplyToEventId, formattedCaption = formattedCaption,
) inReplyToEventId = inReplyToEventId,
)
}
} }
.handleSendResult(mediaId(uri)) .handleSendResult(mediaId(uri))
} }
@@ -158,12 +160,14 @@ class DefaultMediaSender(
audioInfo = audioInfo, audioInfo = audioInfo,
waveform = waveForm, waveform = waveForm,
) )
getTimeline().getOrThrow().sendMedia( withTimeline { timeline ->
uploadInfo = newInfo, timeline.sendMedia(
caption = null, uploadInfo = newInfo,
formattedCaption = null, caption = null,
inReplyToEventId = inReplyToEventId, formattedCaption = null,
) inReplyToEventId = inReplyToEventId,
)
}
} }
.handleSendResult(mediaId(uri)) .handleSendResult(mediaId(uri))
} }
@@ -176,7 +180,7 @@ class DefaultMediaSender(
): Result<Unit> { ): Result<Unit> {
val galleryLogId = "gallery[${mediaUploadInfos.size} items]" val galleryLogId = "gallery[${mediaUploadInfos.size} items]"
Timber.d("Sending $galleryLogId") Timber.d("Sending $galleryLogId")
return getTimeline().flatMap { timeline -> return withTimeline { timeline ->
val galleryItems = mediaUploadInfos.map { it.toGalleryItemInfo() } val galleryItems = mediaUploadInfos.map { it.toGalleryItemInfo() }
timeline.sendGallery( timeline.sendGallery(
items = galleryItems, items = galleryItems,
@@ -184,10 +188,10 @@ class DefaultMediaSender(
formattedCaption = formattedCaption, formattedCaption = formattedCaption,
inReplyToEventId = inReplyToEventId, inReplyToEventId = inReplyToEventId,
) )
.flatMapCatching { uploadHandler ->
uploadHandler.await()
}
} }
.flatMapCatching { uploadHandler ->
uploadHandler.await()
}
.handleSendResult(galleryLogId) .handleSendResult(galleryLogId)
} }
@@ -269,12 +273,23 @@ class DefaultMediaSender(
} }
} }
private suspend fun getTimeline(): Result<Timeline> { /**
* Invokes [block] with the [Timeline] matching [timelineMode].
*
* For a thread, a dedicated timeline is created and closed once [block] returns, since such a
* timeline owns SDK resources. [block] must then perform the whole sending, including waiting
* for the media upload to complete. For the other modes, the live timeline is used, and is not
* closed here since it is owned by its room.
*/
private suspend fun <T> withTimeline(block: suspend (Timeline) -> Result<T>): Result<T> {
return when (timelineMode) { return when (timelineMode) {
is Timeline.Mode.Thread -> { is Timeline.Mode.Thread -> {
room.createTimeline(CreateTimelineParams.Threaded(threadRootEventId = timelineMode.threadRootId)) room.createTimeline(CreateTimelineParams.Threaded(threadRootEventId = timelineMode.threadRootId))
.flatMap { threadedTimeline ->
threadedTimeline.use { block(it) }
}
} }
else -> Result.success(room.liveTimeline) else -> block(room.liveTimeline)
} }
} }
@@ -16,6 +16,7 @@ import io.element.android.libraries.matrix.api.media.FileInfo
import io.element.android.libraries.matrix.api.media.ImageInfo import io.element.android.libraries.matrix.api.media.ImageInfo
import io.element.android.libraries.matrix.api.room.JoinedRoom import io.element.android.libraries.matrix.api.room.JoinedRoom
import io.element.android.libraries.matrix.api.timeline.Timeline import io.element.android.libraries.matrix.api.timeline.Timeline
import io.element.android.libraries.matrix.test.A_THREAD_ID
import io.element.android.libraries.matrix.test.media.FakeMediaUploadHandler import io.element.android.libraries.matrix.test.media.FakeMediaUploadHandler
import io.element.android.libraries.matrix.test.room.FakeJoinedRoom import io.element.android.libraries.matrix.test.room.FakeJoinedRoom
import io.element.android.libraries.matrix.test.timeline.FakeTimeline import io.element.android.libraries.matrix.test.timeline.FakeTimeline
@@ -156,14 +157,52 @@ class DefaultMediaSenderTest : RobolectricTest() {
sendFileResult.assertions().isCalledOnce() sendFileResult.assertions().isCalledOnce()
} }
@Test
fun `given a thread, the created timeline is closed once the media is sent`() = runTest {
val threadedTimeline = FakeTimeline(mode = Timeline.Mode.Thread(A_THREAD_ID)).apply {
sendFileLambda = { _, _, _, _, _ -> Result.success(FakeMediaUploadHandler()) }
}
val liveTimeline = FakeTimeline()
val room = FakeJoinedRoom(
liveTimeline = liveTimeline,
createTimelineResult = { Result.success(threadedTimeline) },
)
val sender = createDefaultMediaSender(
room = room,
timelineMode = Timeline.Mode.Thread(A_THREAD_ID),
)
val uri = Uri.parse("content://image.jpg")
val result = sender.sendMedia(uri = uri, mimeType = MimeTypes.Jpeg, mediaOptimizationConfig = mediaOptimizationConfig)
assertThat(result.isSuccess).isTrue()
assertThat(threadedTimeline.closeCounter).isEqualTo(1)
assertThat(liveTimeline.closeCounter).isEqualTo(0)
}
@Test
fun `given a live timeline, the timeline is not closed once the media is sent`() = runTest {
val liveTimeline = FakeTimeline().apply {
sendFileLambda = { _, _, _, _, _ -> Result.success(FakeMediaUploadHandler()) }
}
val sender = createDefaultMediaSender(room = FakeJoinedRoom(liveTimeline = liveTimeline))
val uri = Uri.parse("content://image.jpg")
val result = sender.sendMedia(uri = uri, mimeType = MimeTypes.Jpeg, mediaOptimizationConfig = mediaOptimizationConfig)
assertThat(result.isSuccess).isTrue()
assertThat(liveTimeline.closeCounter).isEqualTo(0)
}
private fun createDefaultMediaSender( private fun createDefaultMediaSender(
preProcessor: MediaPreProcessor = FakeMediaPreProcessor(), preProcessor: MediaPreProcessor = FakeMediaPreProcessor(),
room: JoinedRoom = FakeJoinedRoom(), room: JoinedRoom = FakeJoinedRoom(),
timelineMode: Timeline.Mode = Timeline.Mode.Live,
mediaOptimizationConfigProvider: MediaOptimizationConfigProvider = MediaOptimizationConfigProvider { mediaOptimizationConfig }, mediaOptimizationConfigProvider: MediaOptimizationConfigProvider = MediaOptimizationConfigProvider { mediaOptimizationConfig },
) = DefaultMediaSender( ) = DefaultMediaSender(
preProcessor = preProcessor, preProcessor = preProcessor,
room = room, room = room,
timelineMode = Timeline.Mode.Live, timelineMode = timelineMode,
mediaOptimizationConfigProvider = mediaOptimizationConfigProvider, mediaOptimizationConfigProvider = mediaOptimizationConfigProvider,
) )
} }