Skip to content

fix: clear LiveKit key debounce timestamp on dispose#2400

Open
marco-a-affinidi wants to merge 1 commit into
famedly:mainfrom
marco-a-affinidi:prevent_stack_overflow
Open

fix: clear LiveKit key debounce timestamp on dispose#2400
marco-a-affinidi wants to merge 1 commit into
famedly:mainfrom
marco-a-affinidi:prevent_stack_overflow

Conversation

@marco-a-affinidi

Copy link
Copy Markdown

If a LiveKit backend is reused within makeKeyOnJoinDelay after dispose, preShareKey() may try to resend a recent key even though local key storage was already cleared. That null-key reuse path recurses through key generation and sending until it overflows the stack.

Reset _lastNewKeyTime on dispose so rejoining generates a fresh key.

@cursor

cursor Bot commented Jul 8, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Narrow VoIP E2EE lifecycle fix on dispose plus a regression test; no auth or broader API changes.

Overview
Fixes a stack overflow when the same LiveKitBackend is reused soon after dispose() and preShareKey() runs again.

dispose() already clears local encryption key storage and index state, but _lastNewKeyTime was left unchanged. Inside makeKeyOnJoinDelay, _makeNewSenderKey could still treat the last key as “recent” and try to resend it via _sendEncryptionKeysEvent. With no local key material, that path falls back to _makeNewSenderKey again and recurses until the stack overflows.

dispose() now resets _lastNewKeyTime to DateTime(1900), so a rejoin always goes through fresh key generation instead of resending a removed key.

Adds livekit_backend_test.dart to assert that after dispose and preShareKey on a new group call, latestLocalKeyIndex advances to 1 and the key provider sees indices [0, 1].

Reviewed by Cursor Bugbot for commit f7c27eb. Bugbot is set up for automated code reviews on this repo. Configure here.

@CLAassistant

CLAassistant commented Jul 8, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@td-famedly td-famedly left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

good catch!

@td-famedly td-famedly enabled auto-merge July 9, 2026 08:37
@codecov

codecov Bot commented Jul 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.60%. Comparing base (f9e6b68) to head (79c0abe).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2400      +/-   ##
==========================================
+ Coverage   59.29%   59.60%   +0.31%     
==========================================
  Files         161      161              
  Lines       20300    20305       +5     
==========================================
+ Hits        12037    12103      +66     
+ Misses       8263     8202      -61     
Files with missing lines Coverage Δ
lib/src/voip/backend/livekit_backend.dart 27.18% <100.00%> (+27.18%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9e6b68...79c0abe. Read the comment docs.

@marco-a-affinidi

Copy link
Copy Markdown
Author

good catch!

Thanks @td-famedly, looks like auto-merge did not work, do I need to do anything on my side?

@td-famedly

Copy link
Copy Markdown
Member

@marco-a-affinidi looks like it needs a rebase so you are not outdated with main

If a LiveKit backend is reused within `makeKeyOnJoinDelay` after dispose,
`preShareKey()` may try to resend a recent key even though local key storage was
already cleared. That null-key reuse path recurses through key generation and
sending until it overflows the stack.

Reset `_lastNewKeyTime` on dispose so rejoining generates a fresh key.
auto-merge was automatically disabled July 9, 2026 13:11

Head branch was pushed to by a user without write access

@marco-a-affinidi marco-a-affinidi force-pushed the prevent_stack_overflow branch from 79c0abe to f7c27eb Compare July 9, 2026 13:11
@marco-a-affinidi

Copy link
Copy Markdown
Author

Thanks @td-famedly, it's now rebased

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.

3 participants