TimelineEvent : fix memory leak

[ganfra]

I spotted this potential memory leak while digging performance issues.

[github-actions]

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/UFbt7u

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (3d0c5e7) 57.62% compared to head (4b6a44d) 57.61%.

❗ Current head 4b6a44d differs from pull request most recent head 5570999. Consider uploading reports for the commit 5570999 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1273      +/-   ##
===========================================
- Coverage    57.62%   57.61%   -0.01%     
===========================================
  Files         1087     1087              
  Lines        28556    28558       +2     
  Branches      5852     5852              
===========================================
  Hits         16454    16454              
- Misses        9533     9535       +2     
  Partials      2569     2569              

Files Changed	Coverage Δ
.../timeline/item/event/TimelineEventContentMapper.kt	0.00% <0.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jmartinesp]

Those are actually all data classes, can they leak something? They don't have an underlying pointer AFAIK.

They're mapped in:

public object FfiConverterTypeTimelineItemContentKind : FfiConverterRustBuffer<TimelineItemContentKind>{
    override fun read(buf: ByteBuffer): TimelineItemContentKind {
        return when(buf.getInt()) {
            1 -> TimelineItemContentKind.Message
            2 -> TimelineItemContentKind.RedactedMessage
            3 -> TimelineItemContentKind.Sticker(
                FfiConverterString.read(buf),
                FfiConverterTypeImageInfo.read(buf),
                FfiConverterString.read(buf),
            )
            ...

[ganfra]

Actually I'm not sure, as they implement Disposable interface, but maybe you're right, I don't know...

sealed class TimelineItemContentKind: Disposable  {
    object Message : TimelineItemContentKind()
    
    object RedactedMessage : TimelineItemContentKind()
    
    data class Sticker(
        val `body`: String, 
        val `info`: ImageInfo, 
        val `url`: String
        ) : TimelineItemContentKind()
    data class Poll(
        val `question`: String, 
        val `kind`: PollKind, 
        val `maxSelections`: ULong, 
        val `answers`: List<PollAnswer>, 
        val `votes`: Map<String, List<String>>, 
        val `endTime`: ULong?
        ) : TimelineItemContentKind()
    data class UnableToDecrypt(
        val `msg`: EncryptedMessage
        ) : TimelineItemContentKind()
    data class RoomMembership(
        val `userId`: String, 
        val `change`: MembershipChange?
        ) : TimelineItemContentKind()
    data class ProfileChange(
        val `displayName`: String?, 
        val `prevDisplayName`: String?, 
        val `avatarUrl`: String?, 
        val `prevAvatarUrl`: String?
        ) : TimelineItemContentKind()
    data class State(
        val `stateKey`: String, 
        val `content`: OtherState
        ) : TimelineItemContentKind()
    data class FailedToParseMessageLike(
        val `eventType`: String, 
        val `error`: String
        ) : TimelineItemContentKind()
    data class FailedToParseState(
        val `eventType`: String, 
        val `stateKey`: String, 
        val `error`: String
        ) : TimelineItemContentKind()
    

    
    @Suppress("UNNECESSARY_SAFE_CALL") // codegen is much simpler if we unconditionally emit safe calls here
    override fun destroy() {
        when(this) {
            is TimelineItemContentKind.Message -> {// Nothing to destroy
            }
            is TimelineItemContentKind.RedactedMessage -> {// Nothing to destroy
            }
            is TimelineItemContentKind.Sticker -> {
                
    Disposable.destroy(
        this.`body`, 
        this.`info`, 
        this.`url`)
                
            }
            is TimelineItemContentKind.Poll -> {
                
    Disposable.destroy(
        this.`question`, 
        this.`kind`, 
        this.`maxSelections`, 
        this.`answers`, 
        this.`votes`, 
        this.`endTime`)
                
            }
            is TimelineItemContentKind.UnableToDecrypt -> {
                
    Disposable.destroy(
        this.`msg`)
                
            }
            is TimelineItemContentKind.RoomMembership -> {
                
    Disposable.destroy(
        this.`userId`, 
        this.`change`)
                
            }
            is TimelineItemContentKind.ProfileChange -> {
                
    Disposable.destroy(
        this.`displayName`, 
        this.`prevDisplayName`, 
        this.`avatarUrl`, 
        this.`prevAvatarUrl`)
                
            }
            is TimelineItemContentKind.State -> {
                
    Disposable.destroy(
        this.`stateKey`, 
        this.`content`)
                
            }
            is TimelineItemContentKind.FailedToParseMessageLike -> {
                
    Disposable.destroy(
        this.`eventType`, 
        this.`error`)
                
            }
            is TimelineItemContentKind.FailedToParseState -> {
                
    Disposable.destroy(
        this.`eventType`, 
        this.`stateKey`, 
        this.`error`)
                
            }
        }.let { /* this makes the `when` an expression, which ensures it is exhaustive */ }
    }
    
}

[jmartinesp]

Actually, I think you're right. The classes themselves are data classes, but some of their properties do have references to Rust pointers, that's why they're closeable. I'll test it tomorrow and approve if everything's fine. Thanks!

[jmartinesp]

LGTM, thanks.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information