Fix a few FFI leaks (#405)

Fix a few FFI leaks These are instances where we obtain an FFIObject and don't call Close on it to release the underlying reference on the Rust side. The worst instance here was leaking an object per room member every time we refreshed the member list
2023-05-12 11:50:39 +01:00
parent 89b9db3be6
commit 92e9d3a127
11 changed files with 67 additions and 59 deletions
@@ -0,0 +1,5 @@
+appId: ${APP_ID}
+---
+- extendedWaitUntil:
+    visible: ${ROOM_NAME}
+    timeout: 10_000
@@ -1,5 +1,6 @@
 appId: ${APP_ID}
 ---
+- runFlow: ../assertions/assertRoomListSynced.yaml
 - tapOn: "search"
 - inputText: ${ROOM_NAME.substring(0, 3)}
 - takeScreenshot: build/maestro/400-SearchRoom
@@ -65,11 +65,12 @@ class CreateRoomRootPresenter @Inject constructor(

        fun startDm(matrixUser: MatrixUser) {
            startDmAction.value = Async.Uninitialized
-            val existingDM = matrixClient.findDM(matrixUser.userId)
-            if (existingDM == null) {
-                localCoroutineScope.createDM(matrixUser, startDmAction)
-            } else {
-                startDmAction.value = Async.Success(existingDM.roomId)
+            matrixClient.findDM(matrixUser.userId).use { existingDM ->
+                if (existingDM == null) {
+                    localCoroutineScope.createDM(matrixUser, startDmAction)
+                } else {
+                    startDmAction.value = Async.Success(existingDM.roomId)
+                }
            }
        }

@@ -131,14 +131,18 @@ class InviteListPresenter @Inject constructor(

    private fun CoroutineScope.acceptInvite(roomId: RoomId, acceptedAction: MutableState<Async<RoomId>>) = launch {
        suspend {
-            client.getRoom(roomId)?.acceptInvitation()?.getOrThrow()
+            client.getRoom(roomId)?.use {
+                it.acceptInvitation().getOrThrow()
+            }
            roomId
        }.execute(acceptedAction)
    }

    private fun CoroutineScope.declineInvite(roomId: RoomId, declinedAction: MutableState<Async<Unit>>) = launch {
        suspend {
-            client.getRoom(roomId)?.rejectInvitation()?.getOrThrow() ?: Unit
+            client.getRoom(roomId)?.use {
+                it.rejectInvitation().getOrThrow()
+            } ?: Unit
        }.execute(declinedAction)
    }

@@ -24,17 +24,18 @@ import org.matrix.rustcomponents.sdk.RoomMember as RustRoomMember

 object RoomMemberMapper {

-    fun map(roomMember: RustRoomMember): RoomMember =
+    fun map(roomMember: RustRoomMember): RoomMember = roomMember.use {
        RoomMember(
-            UserId(roomMember.userId()),
-            roomMember.displayName(),
-            roomMember.avatarUrl(),
-            mapMembership(roomMember.membership()),
-            roomMember.isNameAmbiguous(),
-            roomMember.powerLevel(),
-            roomMember.normalizedPowerLevel(),
-            roomMember.isIgnored(),
+            UserId(it.userId()),
+            it.displayName(),
+            it.avatarUrl(),
+            mapMembership(it.membership()),
+            it.isNameAmbiguous(),
+            it.powerLevel(),
+            it.normalizedPowerLevel(),
+            it.isIgnored(),
        )
+    }

    fun mapMembership(membershipState: RustMembershipState): RoomMembershipState =
        when (membershipState) {
@@ -161,9 +161,10 @@ class RustMatrixRoom(

    override suspend fun sendMessage(message: String): Result<Unit> = withContext(coroutineDispatchers.io) {
        val transactionId = genTransactionId()
-        val content = messageEventContentFromMarkdown(message)
-        runCatching {
-            innerRoom.send(content, transactionId)
+        messageEventContentFromMarkdown(message).use { content ->
+            runCatching {
+                innerRoom.send(content, transactionId)
+            }
        }
    }

@@ -27,12 +27,12 @@ class MatrixTimelineItemMapper(
 ) {

    fun map(timelineItem: TimelineItem): MatrixTimelineItem = timelineItem.use {
-        val asEvent = timelineItem.asEvent()
+        val asEvent = it.asEvent()
        if (asEvent != null) {
            val eventTimelineItem = eventTimelineItemMapper.map(asEvent)
            return MatrixTimelineItem.Event(eventTimelineItem)
        }
-        val asVirtual = timelineItem.asVirtual()
+        val asVirtual = it.asVirtual()
        if (asVirtual != null) {
            val virtualTimelineItem = virtualTimelineItemMapper.map(asVirtual)
            return MatrixTimelineItem.Virtual(virtualTimelineItem)
@@ -39,7 +39,7 @@ import org.matrix.rustcomponents.sdk.MessageFormat as RustMessageFormat
 class EventMessageMapper {

    fun map(message: Message): MessageContent = message.use {
-        val type = message.msgtype().use { type ->
+        val type = it.msgtype().use { type ->
            when (type) {
                is MessageType.Audio -> {
                    AudioMessageType(type.content.body, type.content.source.useUrl(), type.content.info?.map())
@@ -68,9 +68,9 @@ class EventMessageMapper {
            }
        }
        MessageContent(
-            body = message.body(),
-            inReplyTo = message.inReplyTo()?.eventId?.let(::EventId),
-            isEdited = message.isEdited(),
+            body = it.body(),
+            inReplyTo = it.inReplyTo()?.eventId?.let(::EventId),
+            isEdited = it.isEdited(),
            type = type
        )
    }
@@ -31,18 +31,18 @@ class EventTimelineItemMapper(private val contentMapper: TimelineEventContentMap

    fun map(eventTimelineItem: RustEventTimelineItem): EventTimelineItem = eventTimelineItem.use {
        EventTimelineItem(
-            uniqueIdentifier = eventTimelineItem.uniqueIdentifier(),
-            eventId = eventTimelineItem.eventId()?.let { EventId(it) },
-            isEditable = eventTimelineItem.isEditable(),
-            isLocal = eventTimelineItem.isLocal(),
-            isOwn = eventTimelineItem.isOwn(),
-            isRemote = eventTimelineItem.isRemote(),
-            localSendState = eventTimelineItem.localSendState()?.map(),
-            reactions = eventTimelineItem.reactions().map(),
-            sender = UserId(eventTimelineItem.sender()),
-            senderProfile = eventTimelineItem.senderProfile().map(),
-            timestamp = eventTimelineItem.timestamp().toLong(),
-            content = contentMapper.map(eventTimelineItem.content())
+            uniqueIdentifier = it.uniqueIdentifier(),
+            eventId = it.eventId()?.let { EventId(it) },
+            isEditable = it.isEditable(),
+            isLocal = it.isLocal(),
+            isOwn = it.isOwn(),
+            isRemote = it.isRemote(),
+            localSendState = it.localSendState()?.map(),
+            reactions = it.reactions().map(),
+            sender = UserId(it.sender()),
+            senderProfile = it.senderProfile().map(),
+            timestamp = it.timestamp().toLong(),
+            content = contentMapper.map(it.content())
        )
    }
 }
@@ -17,6 +17,7 @@
 package io.element.android.libraries.matrix.impl.timeline.item.event

 import io.element.android.libraries.matrix.api.core.UserId
+import io.element.android.libraries.matrix.api.timeline.item.event.EventContent
 import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseMessageLikeContent
 import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParseStateContent
 import io.element.android.libraries.matrix.api.timeline.item.event.MembershipChange
@@ -26,7 +27,6 @@ import io.element.android.libraries.matrix.api.timeline.item.event.RedactedConte
 import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StateContent
 import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent
-import io.element.android.libraries.matrix.api.timeline.item.event.EventContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent
 import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent
 import io.element.android.libraries.matrix.impl.media.map
@@ -39,7 +39,7 @@ import org.matrix.rustcomponents.sdk.OtherState as RustOtherState
 class TimelineEventContentMapper(private val eventMessageMapper: EventMessageMapper = EventMessageMapper()) {

    fun map(content: TimelineItemContent): EventContent = content.use {
-        when (val kind = content.kind()) {
+        when (val kind = it.kind()) {
            is TimelineItemContentKind.FailedToParseMessageLike -> {
                FailedToParseMessageLikeContent(
                    eventType = kind.eventType,
@@ -54,7 +54,7 @@ class TimelineEventContentMapper(private val eventMessageMapper: EventMessageMap
                )
            }
            TimelineItemContentKind.Message -> {
-                val message = content.asMessage()
+                val message = it.asMessage()
                if (message == null) {
                    UnknownContent
                } else {
@@ -33,17 +33,16 @@ import io.element.android.libraries.dateformatter.impl.LocalDateTimeProvider
 import io.element.android.libraries.designsystem.utils.SnackbarDispatcher
 import io.element.android.libraries.matrix.api.MatrixClient
 import io.element.android.libraries.matrix.api.core.RoomId
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.asCoroutineDispatcher
 import kotlinx.coroutines.launch
+import kotlinx.coroutines.withContext
 import kotlinx.datetime.Clock
 import kotlinx.datetime.TimeZone
 import java.util.Locale
-import java.util.concurrent.Executors

 class RoomListScreen(
    context: Context,
    private val matrixClient: MatrixClient,
+    private val coroutineDispatchers: CoroutineDispatchers = Singleton.coroutineDispatchers,
 ) {
    private val clock = Clock.System
    private val locale = Locale.getDefault()
@@ -58,28 +57,24 @@ class RoomListScreen(
        sessionVerificationService = sessionVerificationService,
        networkMonitor = NetworkMonitorImpl(context),
        snackbarDispatcher = SnackbarDispatcher(),
-        inviteStateDataSource = DefaultInviteStateDataSource(
-            matrixClient,
-            DefaultSeenInvitesStore(context),
-            CoroutineDispatchers(
-                io = Dispatchers.IO,
-                computation = Dispatchers.Default,
-                main = Dispatchers.Main,
-                diffUpdateDispatcher = Executors.newSingleThreadExecutor().asCoroutineDispatcher()
-            )
-        )
+        inviteStateDataSource = DefaultInviteStateDataSource(matrixClient, DefaultSeenInvitesStore(context), coroutineDispatchers)
    )

    @Composable
    fun Content(modifier: Modifier = Modifier) {
        fun onRoomClicked(roomId: RoomId) {
-            val room = matrixClient.getRoom(roomId)!!
-            val timeline = room.timeline()
            Singleton.appScope.launch {
-                timeline.apply {
-                    initialize()
-                    paginateBackwards(20, 50)
-                    dispose()
+                withContext(coroutineDispatchers.io) {
+                    matrixClient.getRoom(roomId)!!.use { room ->
+                        val timeline = room.timeline()
+
+                        timeline.apply {
+                            // TODO This doesn't work reliably as initialize is asynchronous, and the timeline can't be used until it's finished
+                            initialize()
+                            paginateBackwards(20, 50)
+                            dispose()
+                        }
+                    }
                }
            }
        }