Skip to content

Fix Two Memory Leaks in RoomSession and DataChannel - #1377

Open
raparanjpe wants to merge 3 commits into
livekit:mainfrom
raparanjpe:main
Open

Fix Two Memory Leaks in RoomSession and DataChannel#1377
raparanjpe wants to merge 3 commits into
livekit:mainfrom
raparanjpe:main

Conversation

@raparanjpe

Copy link
Copy Markdown
Contributor

This PR fixes two leaks, one with an ArcCycle between a Room and itself due to passing a strong reference to the RoomSession into a closure (on_state_changed contained by the RoomSession). For this we simply downgrade the Arc in the same way we do above on line 758 in mod.rs for the local_participant.

For the second leak we noticed a DataChannel cycle triggered via the forward_dc_events call in rtc_events.rs, specifically dc.on_buffered_amount_change(Some(on_buffered_amount_change(rtc_emitter, dc.clone(), kind))); (i.e. we're cloning the dc and passing it into itself, creating the strong reference cycle).

Initially we were going to just pass the amount from dc directly, but noticed we don't actually need that in the struct, so opted to remove the field directly. If it's needed for other reasons we can add it back without cloning the data channel arc.

The leak is proportional to room joins, and so we set up a simple rig to test joining and leaving rooms to measure the leak amount via jemalloc's stats.allocated gauge and got these results:

Build Per Room Join (kb)
main 162
PR 20

Although this is a bit minimal, over many calls a day the leak accumulates quite a bit.

@raparanjpe
raparanjpe requested a review from ladvoc as a code owner August 30, 2026 20:36
);
}
}
RtcEvent::DataChannelBufferedAmountChange { sent, amount: _, kind } => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the only place I was able to see amount being used and it's being ignored, so I opted to remove amount instead of wiring it through from the data channel.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

@CLAassistant

CLAassistant commented Aug 30, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants