The closure captures the bound instance method processPresenceSubscribe, which implicitly retains self.
If the caller forgets to call unsubscribe(), the underlying Ably listener will keep the closure – and therefore the Implementation instance – alive indefinitely.
Refactor the capture list so the closure holds a weak reference to self and invokes the helper via that weak reference:
-let eventListener = channel.presence.subscribe(event.toARTPresenceAction()) { [processPresenceSubscribe, logger] message in
- logger.log(message: "Received presence message: \(message)", level: .debug)
- do {
- // processPresenceSubscribe is logging so we don't need to log here
- let presenceEvent = try processPresenceSubscribe(PresenceMessage(ablyCocoaPresenceMessage: message), event)
- callback(presenceEvent)
- } catch {
- // note: this replaces some existing code that also didn't handle the processPresenceSubscribe error; I suspect not intentional, will leave whoever writes the tests for this class to see what's going on
- }
+let eventListener = channel.presence.subscribe(event.toARTPresenceAction()) { [weak self, logger] message in
+ guard let self else { return }
+ logger.log(message: "Received presence message: \(message)", level: .debug)
+ do {
+ let presenceEvent = try self.processPresenceSubscribe(
+ PresenceMessage(ablyCocoaPresenceMessage: message),
+ for: event
+ )
+ callback(presenceEvent)
+ } catch {
+ logger.log(message: "Error processing presence: \(error)", level: .error)
+ }
}
This eliminates the retain cycle and restores error logging.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
let eventListener = channel.presence.subscribe(event.toARTPresenceAction()) { [weak self, logger] message in
guard let self else { return }
logger.log(message: "Received presence message: \(message)", level: .debug)
do {
let presenceEvent = try self.processPresenceSubscribe(
PresenceMessage(ablyCocoaPresenceMessage: message),
for: event
)
callback(presenceEvent)
} catch {
logger.log(message: "Error processing presence: \(error)", level: .error)
}
}
🤖 Prompt for AI Agents (early access)
In Sources/AblyChat/DefaultPresence.swift around lines 269 to 277, the closure
captures the instance method processPresenceSubscribe strongly, causing a retain
cycle that keeps self alive if unsubscribe() is not called. To fix this, change
the capture list to hold a weak reference to self, then call
processPresenceSubscribe through this weak reference inside the closure. This
prevents the retain cycle and ensures error logging is restored.
Originally posted by @coderabbitai[bot] in #286 (comment)
┆Issue is synchronized with this Jira Task by Unito
The closure captures the bound instance method
processPresenceSubscribe, which implicitly retainsself.If the caller forgets to call
unsubscribe(), the underlying Ably listener will keep the closure – and therefore theImplementationinstance – alive indefinitely.Refactor the capture list so the closure holds a weak reference to
selfand invokes the helper via that weak reference:This eliminates the retain cycle and restores error logging.
📝 Committable suggestion
🤖 Prompt for AI Agents (early access)
Originally posted by @coderabbitai[bot] in #286 (comment)
┆Issue is synchronized with this Jira Task by Unito