-
Notifications
You must be signed in to change notification settings - Fork 163
Fix Task cancellation leaks #874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
71653c5 to
38c581f
Compare
6995459 to
3d35294
Compare
338bd0a to
86fae4a
Compare
77fe34a to
522ed78
Compare
| /// - onFailure: Called when the sequence terminates with an error. Cancellation errors are ignored. | ||
| /// - Returns: The task cancellable. | ||
| func subscribe<O: AnyObject & Sendable, State: Sendable>( | ||
| _ observer: O, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I'll add some more descriptive label like subscribe(weakly: self) etc.
| if let subscriber { self.noLeaks(of: subscriber) } | ||
|
|
||
| self.noLeaks(of: room.publisherDataChannel) | ||
| self.noLeaks(of: room.subscriberDataChannel) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hiroshihorie I compared with main again and confirmed that the main leak comes from DC (its main event loop).
c6fbc01 to
abf7230
Compare
|
Linking this one for reference: https://medium.com/the-swift-cooperative/using-async-for-await-youre-probably-doing-it-wrong-88b66fbb0e84 |
# Conflicts: # Sources/LiveKit/Core/DataChannelPair.swift
|
FYI, I added a brand-new SwiftLint rule to prevent bare |
Resolves #866
Fixes remaining concurrency antipatterns (mostly implicit
selfinside a long-runningTask) so thatdeinit(and cancel etc.) is never called.Introduces
AnyTaskCancellablewrapper for more Combine-like semantics (RAII).