Skip to content

Commit a97b5aa

Browse files
AndyTWFAndyTWF
committed
Add dispose() method to ChatClient for resource cleanup
Implements CHA-CL1 spec: adds dispose() method that releases all resources held by the ChatClient instance. - ChatClient.dispose() releases all rooms (CHA-CL1a) then disposes connection (CHA-CL1b) - DefaultRooms.dispose() releases all managed rooms concurrently - DefaultConnection.dispose() removes all event listeners - All dispose methods are idempotent - Added MockChatClient.dispose() for example app compatibility - Mark DefaultConnection as @mainactor and wrap Ably callback for thread safety
1 parent 53b120d commit a97b5aa

File tree

13 files changed

+424
-43
lines changed

13 files changed

+424
-43
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ DerivedData/
1111

1212
/node_modules
1313
/.mint
14+
.claude/settings.local.json

‎Example/AblyChatExample/Mocks/MockClients.swift‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ class MockChatClient: ChatClientProtocol {
2020
var clientID: String? {
2121
"AblyTest"
2222
}
23+
24+
func dispose() async {
25+
// No-op for mock
26+
}
2327
}
2428

2529
class MockRooms: Rooms {

‎Sources/AblyChat/ChatClient.swift‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,20 @@ public protocol ChatClientProtocol: AnyObject, Sendable {
5353
* - Returns: The client options.
5454
*/
5555
var clientOptions: ChatClientOptions { get }
56+
57+
/**
58+
* Disposes of the chat client and releases all associated resources.
59+
*
60+
* Calling this method will:
61+
* - Release all rooms managed by this client
62+
* - Clean up connection listeners and timers
63+
*
64+
* After calling this method, the client should not be used. Attempting to get a room
65+
* after dispose has been called will throw an error.
66+
*
67+
* This method is idempotent - calling it multiple times has no additional effect.
68+
*/
69+
func dispose() async
5670
}
5771

5872
@MainActor
@@ -131,6 +145,26 @@ public class ChatClient: ChatClientProtocol {
131145
public var clientID: String? {
132146
realtime.clientId
133147
}
148+
149+
/// Whether this ChatClient has been disposed.
150+
private var isDisposed = false
151+
152+
// @spec CHA-CL1
153+
// swiftlint:disable:next missing_docs
154+
public func dispose() async {
155+
// Idempotency check
156+
if isDisposed {
157+
return
158+
}
159+
160+
isDisposed = true
161+
162+
// CHA-CL1a: First, dispose of all rooms
163+
await _rooms.dispose()
164+
165+
// CHA-CL1b: Then, dispose of connection instance
166+
_connection.dispose()
167+
}
134168
}
135169

136170
/**

‎Sources/AblyChat/DefaultConnection.swift‎

Lines changed: 84 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,16 @@
11
import Ably
22

3+
@MainActor
34
internal final class DefaultConnection: Connection {
45
private let realtime: any InternalRealtimeClientProtocol
56
private let timerManager = TimerManager(clock: SystemClock())
67

8+
/// Track all event listeners we've created so we can clean them up on dispose.
9+
private var eventListeners: [ARTEventListener] = []
10+
11+
/// Whether this Connection instance has been disposed.
12+
private var isDisposed = false
13+
714
// (CHA-CS2a) The chat client must expose its current connection status.
815
internal var status: ConnectionStatus {
916
.fromRealtimeConnectionState(realtime.connection.state)
@@ -24,61 +31,97 @@ internal final class DefaultConnection: Connection {
2431
internal func onStatusChange(_ callback: @escaping @MainActor (ConnectionStatusChange) -> Void) -> some StatusSubscription {
2532
// (CHA-CS5) The chat client must monitor the underlying realtime connection for connection status changes.
2633
let eventListener = realtime.connection.on { [weak self] stateChange in
27-
guard let self else {
28-
return
29-
}
30-
let currentState = ConnectionStatus.fromRealtimeConnectionState(stateChange.current)
31-
let previousState = ConnectionStatus.fromRealtimeConnectionState(stateChange.previous)
32-
33-
// (CHA-CS4a) Connection status update events must contain the newly entered connection status.
34-
// (CHA-CS4b) Connection status update events must contain the previous connection status.
35-
// (CHA-CS4c) Connection status update events must contain the connection error (if any) that pertains to the newly entered connection status.
36-
let statusChange = ConnectionStatusChange(
37-
current: currentState,
38-
previous: previousState,
39-
error: stateChange.reason,
40-
// TODO: Actually emit `nil` when appropriate (we can't currently since ably-cocoa's corresponding property is mis-typed): https://github.com/ably/ably-chat-swift/issues/394
41-
retryIn: stateChange.retryIn,
42-
)
43-
44-
let isTimerRunning = timerManager.hasRunningTask()
45-
// (CHA-CS5a) The chat client must suppress transient disconnection events. It is not uncommon for Ably servers to perform connection shedding to balance load, or due to retiring. Clients should not need to concern themselves with transient events.
46-
47-
// (CHA-CS5a2) If a transient disconnect timer is active and the realtime connection status changes to `DISCONNECTED` or `CONNECTING`, the library must not emit a status change.
48-
if isTimerRunning, currentState == .disconnected || currentState == .connecting {
49-
return
50-
}
34+
Task { @MainActor in
35+
guard let self else {
36+
return
37+
}
38+
let currentState = ConnectionStatus.fromRealtimeConnectionState(stateChange.current)
39+
let previousState = ConnectionStatus.fromRealtimeConnectionState(stateChange.previous)
5140

52-
// (CHA-CS5a3) If a transient disconnect timer is active and the realtime connections status changes to `CONNECTED`, `SUSPENDED` or `FAILED`, the library shall cancel the transient disconnect timer. The superseding status change shall be emitted.
53-
if isTimerRunning, currentState == .connected || currentState == .suspended || currentState == .failed {
54-
timerManager.cancelTimer()
55-
callback(statusChange)
56-
}
41+
// (CHA-CS4a) Connection status update events must contain the newly entered connection status.
42+
// (CHA-CS4b) Connection status update events must contain the previous connection status.
43+
// (CHA-CS4c) Connection status update events must contain the connection error (if any) that pertains to the newly entered connection status.
44+
let statusChange = ConnectionStatusChange(
45+
current: currentState,
46+
previous: previousState,
47+
error: stateChange.reason,
48+
// TODO: Actually emit `nil` when appropriate (we can't currently since ably-cocoa's corresponding property is mis-typed): https://github.com/ably/ably-chat-swift/issues/394
49+
retryIn: stateChange.retryIn,
50+
)
5751

58-
// (CHA-CS5a1) If the realtime connection status transitions from `CONNECTED` to `DISCONNECTED`, the chat client connection status must not change. A 5 second transient disconnect timer shall be started.
59-
if previousState == .connected, currentState == .disconnected, !isTimerRunning {
60-
timerManager.setTimer(interval: 5.0) { [timerManager] in
61-
// (CHA-CS5a4) If a transient disconnect timer expires the library shall emit a connection status change event. This event must contain the current status of of timer expiry, along with the original error that initiated the transient disconnect timer.
62-
timerManager.cancelTimer()
52+
let isTimerRunning = self.timerManager.hasRunningTask()
53+
// (CHA-CS5a) The chat client must suppress transient disconnection events. It is not uncommon for Ably servers to perform connection shedding to balance load, or due to retiring. Clients should not need to concern themselves with transient events.
54+
55+
// (CHA-CS5a2) If a transient disconnect timer is active and the realtime connection status changes to `DISCONNECTED` or `CONNECTING`, the library must not emit a status change.
56+
if isTimerRunning, currentState == .disconnected || currentState == .connecting {
57+
return
58+
}
59+
60+
// (CHA-CS5a3) If a transient disconnect timer is active and the realtime connections status changes to `CONNECTED`, `SUSPENDED` or `FAILED`, the library shall cancel the transient disconnect timer. The superseding status change shall be emitted.
61+
if isTimerRunning, currentState == .connected || currentState == .suspended || currentState == .failed {
62+
self.timerManager.cancelTimer()
6363
callback(statusChange)
6464
}
65-
return
66-
}
6765

68-
if isTimerRunning {
69-
timerManager.cancelTimer()
70-
}
66+
// (CHA-CS5a1) If the realtime connection status transitions from `CONNECTED` to `DISCONNECTED`, the chat client connection status must not change. A 5 second transient disconnect timer shall be started.
67+
if previousState == .connected, currentState == .disconnected, !isTimerRunning {
68+
self.timerManager.setTimer(interval: 5.0) { [timerManager = self.timerManager] in
69+
// (CHA-CS5a4) If a transient disconnect timer expires the library shall emit a connection status change event. This event must contain the current status of of timer expiry, along with the original error that initiated the transient disconnect timer.
70+
timerManager.cancelTimer()
71+
callback(statusChange)
72+
}
73+
return
74+
}
75+
76+
if isTimerRunning {
77+
self.timerManager.cancelTimer()
78+
}
7179

72-
// (CHA-CS5b) Not withstanding CHA-CS5a. If a connection state event is observed from the underlying realtime library, the client must emit a status change event. The current status of that event shall reflect the status change in the underlying realtime library, along with the accompanying error.
73-
callback(statusChange)
80+
// (CHA-CS5b) Not withstanding CHA-CS5a. If a connection state event is observed from the underlying realtime library, the client must emit a status change event. The current status of that event shall reflect the status change in the underlying realtime library, along with the accompanying error.
81+
callback(statusChange)
82+
}
7483
}
7584

85+
// Track this listener so we can clean it up on dispose
86+
eventListeners.append(eventListener)
87+
7688
return DefaultStatusSubscription { [weak self] in
7789
guard let self else {
7890
return
7991
}
8092
timerManager.cancelTimer()
8193
realtime.connection.off(eventListener)
94+
// Remove from tracked list
95+
eventListeners.removeAll { $0 === eventListener }
96+
}
97+
}
98+
99+
// @spec CHA-CL1b
100+
/// Disposes of the connection, cleaning up any listeners and timers.
101+
///
102+
/// This method:
103+
/// 1. Cancels any active transient disconnect timers
104+
/// 2. Removes all connection state listeners we've registered
105+
///
106+
/// Note: We do NOT close the underlying realtime connection. The ARTRealtime instance
107+
/// is owned by the user and passed to ChatClient.
108+
///
109+
/// This method is idempotent.
110+
internal func dispose() {
111+
// Idempotency check
112+
if isDisposed {
113+
return
114+
}
115+
116+
isDisposed = true
117+
118+
// Cancel any active transient disconnect timer
119+
timerManager.cancelTimer()
120+
121+
// Remove all connection state listeners we've registered
122+
for listener in eventListeners {
123+
realtime.connection.off(listener)
82124
}
125+
eventListeners.removeAll()
83126
}
84127
}

‎Sources/AblyChat/DefaultMessageReactions.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,4 +150,10 @@ internal final class DefaultMessageReactions<Realtime: InternalRealtimeClientPro
150150

151151
return summary
152152
}
153+
154+
/// Disposes of the message reactions feature.
155+
/// This is a no-op as the feature has no internal state to clean up.
156+
internal func dispose() {
157+
// No internal state to clean up
158+
}
153159
}

‎Sources/AblyChat/DefaultMessages.swift‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,12 @@ internal final class DefaultMessages<Realtime: InternalRealtimeClientProtocol>:
1212
private var currentSubscriptionPoint: String?
1313
private var subscriptionPoints: [UUID: String] = [:]
1414

15+
/// The channel state listener used to track subscription points.
16+
private var channelStateListener: ARTEventListener?
17+
1518
private func updateCurrentSubscriptionPoint() {
1619
currentSubscriptionPoint = channel.properties.attachSerial
17-
_ = channel.on { [weak self] stateChange in
20+
channelStateListener = channel.on { [weak self] stateChange in
1821
guard let self else {
1922
return
2023
}
@@ -175,4 +178,20 @@ internal final class DefaultMessages<Realtime: InternalRealtimeClientProtocol>:
175178
internal enum MessagesError: Error {
176179
case noReferenceToSelf
177180
}
181+
182+
/// Disposes of the messages feature, cleaning up listeners and state.
183+
internal func dispose() {
184+
// Remove channel state listener
185+
if let channelStateListener {
186+
channel.off(channelStateListener)
187+
}
188+
channelStateListener = nil
189+
190+
// Clear subscription points
191+
currentSubscriptionPoint = nil
192+
subscriptionPoints.removeAll()
193+
194+
// Dispose of message reactions
195+
reactions.dispose()
196+
}
178197
}

‎Sources/AblyChat/DefaultOccupancy.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,9 @@ internal final class DefaultOccupancy<Realtime: InternalRealtimeClientProtocol>:
7474
// CHA-07b
7575
return lastOccupancyData
7676
}
77+
78+
/// Disposes of the occupancy feature, clearing cached state.
79+
internal func dispose() {
80+
lastOccupancyData = nil
81+
}
7782
}

‎Sources/AblyChat/DefaultTyping.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,4 +176,9 @@ internal final class DefaultTyping: Typing {
176176
throw error
177177
}
178178
}
179+
180+
/// Disposes of the typing feature, cancelling all timers and clearing state.
181+
internal func dispose() {
182+
typingTimerManager.dispose()
183+
}
179184
}

‎Sources/AblyChat/InternalError.swift‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ internal enum InternalError {
7878
/// Error code is `invalidArgument`.
7979
case unableDeleteReactionWithoutName(reactionType: String)
8080

81+
/// The user tried to get a room from a client that has been disposed, per CHA-CL1.
82+
///
83+
/// Error code is `resourceDisposed`.
84+
case clientDisposed
85+
8186
// MARK: - Errors not from the spec
8287

8388
// TODO: Revisit the non-specified errors as part of https://github.com/ably/ably-chat-swift/issues/438
@@ -130,6 +135,7 @@ internal enum InternalError {
130135
internal enum ErrorCode: Int {
131136
case badRequest = 40000
132137
case invalidArgument = 40003
138+
case resourceDisposed = 40014
133139
case roomDiscontinuity = 102_100
134140
case roomReleasedBeforeOperationCompleted = 102_106
135141
case roomExistsWithDifferentOptions = 102_107
@@ -141,6 +147,7 @@ internal enum InternalError {
141147
switch self {
142148
case .badRequest,
143149
.invalidArgument,
150+
.resourceDisposed,
144151
.roomReleasedBeforeOperationCompleted,
145152
.roomInInvalidState,
146153
.roomExistsWithDifferentOptions:
@@ -197,6 +204,8 @@ internal enum InternalError {
197204
.badRequest
198205
case .jsonValueDecodingError:
199206
.badRequest
207+
case .clientDisposed:
208+
.resourceDisposed
200209
}
201210
}
202211

@@ -322,6 +331,9 @@ internal enum InternalError {
322331
case let .failedToDecodeFromRawValue(type: type, rawValue: rawValue):
323332
reason = "could not decode \(type) from raw value \(rawValue)"
324333
}
334+
case .clientDisposed:
335+
op = "get room"
336+
reason = "client has been disposed"
325337
}
326338

327339
return "unable to \(op); \(reason)"
@@ -353,7 +365,8 @@ internal enum InternalError {
353365
.deleteMessageReactionEmptyMessageSerial,
354366
.noItemInResponse,
355367
.paginatedResultStatusCode,
356-
.failedToResolveSubscriptionPointBecauseMessagesInstanceGone:
368+
.failedToResolveSubscriptionPointBecauseMessagesInstanceGone,
369+
.clientDisposed:
357370
nil
358371
}
359372
}

‎Sources/AblyChat/Room.swift‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,11 @@ internal class DefaultRoom<Realtime: InternalRealtimeClientProtocol, LifecycleMa
386386
internal func release() async {
387387
await lifecycleManager.performReleaseOperation()
388388

389+
// Dispose of room features to clean up any internal state
390+
typing.dispose()
391+
occupancy.dispose()
392+
messages.dispose()
393+
389394
// CHA-RL3h
390395
realtime.channels.release(internalChannel.name)
391396
}

0 commit comments

Comments
 (0)