Fix connection leaks: Ensure that any pending connection open are cancelled/undo when ActiveCall is unmounted #3255
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AudioCaptureOptions was a different object but with same internal values, use directly deviceId so that Object.is works properly
BillCarsonFr
force-pushed
the
valere/fix_connections_leaks
branch
from
b529ebc to
83c630c
Compare
toger5
requested changes
May 14, 2025
src/livekit/useECConnectionState.ts
Outdated
| // Protection against potential leaks, where the component to be unmounted and there is | ||
| // still a pending doConnect promise. This would lead the user to still be in the call even | ||
| // if the component is unmounted. | ||
| const cancelBag = useRef(new Set<Cancellable>()); |
| { | ||
| deviceId: initialDevices.current.audioInput.selectedId, | ||
| }, | ||
| initialDevices.current.audioInput.selectedId, |
There was a problem hiding this comment.
Really good that you detected and fixed this!
src/utils/cancellable.ts
Outdated
Comment on lines
8
to
18
| export class Cancellable { | ||
| public constructor(private cancelled = false) {} | ||
|
|
||
| public cancel(): void { | ||
| this.cancelled = true; | ||
| } | ||
|
|
||
| public isCancelled(): boolean { | ||
| return this.cancelled; | ||
| } | ||
| } |
There was a problem hiding this comment.
Suggested change
| export class Cancellable { | |
| public constructor(private cancelled = false) {} | |
| public cancel(): void { | |
| this.cancelled = true; | |
| } | |
| public isCancelled(): boolean { | |
| return this.cancelled; | |
| } | |
| } | |
| export class AbortHandle { | |
| public constructor(private aborted = false) {} | |
| public abort(): void { | |
| this.aborted = true; | |
| } | |
| public isAborted(): boolean { | |
| return this.aborted; | |
| } | |
| } |
toger5
reviewed
May 14, 2025
toger5
force-pushed
the
valere/fix_connections_leaks
branch
from
a273f34 to
c33fa98
Compare
toger5
approved these changes
May 14, 2025
toger5
added a commit
that referenced
this pull request
May 14, 2025
…celled/undo when ActiveCall is unmounted (#3255) * Better logs for connection/component lifecycle * fix: `AudioCaptureOptions` was causing un-necessary effect render AudioCaptureOptions was a different object but with same internal values, use directly deviceId so that Object.is works properly * fix: Livekit openned connection leaks * review: rename to AbortHandles * review: rename variable --------- Co-authored-by: Timo <[email protected]>
toger5
added a commit
that referenced
this pull request
May 14, 2025
…celled/undo when ActiveCall is unmounted (#3255) (#3269) * Better logs for connection/component lifecycle * fix: `AudioCaptureOptions` was causing un-necessary effect render AudioCaptureOptions was a different object but with same internal values, use directly deviceId so that Object.is works properly * fix: Livekit openned connection leaks * review: rename to AbortHandles * review: rename variable --------- Co-authored-by: Valere Fedronic <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #3268