From 919cf7d220bc70fb31579bade3bca92ff85db03b Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 12 Nov 2024 18:57:31 +0530 Subject: [PATCH 1/4] Added basic impl. for room release as a part of roomLifeCycleManager --- .../com/ably/chat/RoomLifecycleManager.kt | 58 ++++++++++++++++++- .../src/main/java/com/ably/chat/Utils.kt | 7 +++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt index edc34936..548f8aa4 100644 --- a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt +++ b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt @@ -175,6 +175,11 @@ class RoomLifecycleManager( */ private val _firstAttachesCompleted = mutableMapOf() + /** + * Are we in the process of releasing the room? + */ + private var _releaseInProgress = false + /** * Retry duration in milliseconds, used by internal doRetry and runDownChannelsOnFailedAttach methods */ @@ -589,6 +594,57 @@ class RoomLifecycleManager( * state (e.g. attached), release will throw exception. */ internal suspend fun release() { - // TODO("Need to impl. room release") + val deferredRelease = atomicCoroutineScope.async(LifecycleOperationPrecedence.Release.priority) { // CHA-RL2i + // If we're already released, this is a no-op + if (_statusLifecycle.status === RoomStatus.Released) { + return@async + } + + // If we're already detached, then we can transition to released immediately + if (_statusLifecycle.status === RoomStatus.Detached) { + _statusLifecycle.setStatus(RoomStatus.Released) + return@async + } + + // If we're in the process of releasing, we should wait for it to complete + if (_releaseInProgress) { + return@async listenToRoomRelease() + } + + // We force the room status to be releasing + clearAllTransientDetachTimeouts() + _operationInProgress = true + _releaseInProgress = true + _statusLifecycle.setStatus(RoomStatus.Releasing) + + // Do the release until it completes + return@async releaseChannels() + } + deferredRelease.await() + } + + private suspend fun listenToRoomRelease() = suspendCancellableCoroutine { continuation -> + _statusLifecycle.onChangeOnce { + if (it.current == RoomStatus.Released) { + continuation.resume(Unit) + } else { + val err = AblyException.fromErrorInfo( + ErrorInfo( + "failed to release room; existing attempt failed${it.errorMessage}", + HttpStatusCodes.InternalServerError, + ErrorCodes.PreviousOperationFailed.errorCode, + ), + ) + continuation.resumeWithException(err) + } + } + } + + /** + * Releases the room by detaching all channels. If the release operation fails, we wait + * a short period and then try again. + */ + private suspend fun releaseChannels() { + // TODO("need to be implemented") } } diff --git a/chat-android/src/main/java/com/ably/chat/Utils.kt b/chat-android/src/main/java/com/ably/chat/Utils.kt index 41fa2607..434a33dd 100644 --- a/chat-android/src/main/java/com/ably/chat/Utils.kt +++ b/chat-android/src/main/java/com/ably/chat/Utils.kt @@ -57,6 +57,13 @@ val Channel.errorMessage: String ", ${reason.message}" } +val RoomStatusChange.errorMessage: String + get() = if (error == null) { + "" + } else { + ", ${error.message}" + } + @Suppress("FunctionName") fun ChatChannelOptions(init: (ChannelOptions.() -> Unit)? = null): ChannelOptions { val options = ChannelOptions() From 143bea341864158a54be8561dc945182a8fe89e3 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 13 Nov 2024 18:51:12 +0530 Subject: [PATCH 2/4] Added missing impl for releaseChannels and doRelease method as a part of RoomLifecycleManager release --- .../com/ably/chat/RoomLifecycleManager.kt | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt index 548f8aa4..842e1e36 100644 --- a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt +++ b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt @@ -645,6 +645,39 @@ class RoomLifecycleManager( * a short period and then try again. */ private suspend fun releaseChannels() { - // TODO("need to be implemented") + var contributorsReleased = kotlin.runCatching { doRelease() } + while (contributorsReleased.isFailure) { + // Wait a short period and then try again + delay(_retryDurationInMs) + contributorsReleased = kotlin.runCatching { doRelease() } + } + } + + /** + * Performs the release operation. This will detach all channels in the room that aren't + * already detached or in the failed state. + */ + @Suppress("RethrowCaughtException") + private suspend fun doRelease() = coroutineScope { + _contributors.map { contributor: ResolvedContributor -> + async { + // Failed channels, we can ignore + if (contributor.channel.state == ChannelState.failed) { + return@async + } + // Detached channels, we can ignore + if (contributor.channel.state == ChannelState.detached) { + return@async + } + try { + contributor.channel.detachCoroutine() + } catch (ex: Throwable) { + // TODO - log error here before rethrowing + throw ex + } + } + }.awaitAll() + _releaseInProgress = false + _statusLifecycle.setStatus(RoomStatus.Released) } } From 4cac4eb9b39628fbb1b1f56b364663d924f7dfa8 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 13 Nov 2024 23:24:46 +0530 Subject: [PATCH 3/4] Added missing spec impl. for CHA-RL3j as per spec --- .../src/main/java/com/ably/chat/RoomLifecycleManager.kt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt index 842e1e36..08f94c38 100644 --- a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt +++ b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt @@ -601,7 +601,9 @@ class RoomLifecycleManager( } // If we're already detached, then we can transition to released immediately - if (_statusLifecycle.status === RoomStatus.Detached) { + if (_statusLifecycle.status === RoomStatus.Detached || + _statusLifecycle.status === RoomStatus.Initialized + ) { _statusLifecycle.setStatus(RoomStatus.Released) return@async } From ee8fc59e33eff398998c49c9e039d0def48cb3d1 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 15 Nov 2024 16:01:46 +0530 Subject: [PATCH 4/4] Updated interface ContributesToRoomLifecycle, added release() method Added impl. for the release method across all room features --- .../src/main/java/com/ably/chat/Messages.kt | 14 +++++++------- .../src/main/java/com/ably/chat/Occupancy.kt | 4 ++++ .../src/main/java/com/ably/chat/Presence.kt | 4 ++++ chat-android/src/main/java/com/ably/chat/Room.kt | 9 +++++++-- .../java/com/ably/chat/RoomLifecycleManager.kt | 8 ++++++++ .../src/main/java/com/ably/chat/RoomReactions.kt | 9 +++++++-- chat-android/src/main/java/com/ably/chat/Rooms.kt | 6 ++---- chat-android/src/main/java/com/ably/chat/Typing.kt | 4 ++++ 8 files changed, 43 insertions(+), 15 deletions(-) diff --git a/chat-android/src/main/java/com/ably/chat/Messages.kt b/chat-android/src/main/java/com/ably/chat/Messages.kt index 55934579..ee5567ac 100644 --- a/chat-android/src/main/java/com/ably/chat/Messages.kt +++ b/chat-android/src/main/java/com/ably/chat/Messages.kt @@ -223,10 +223,12 @@ internal class DefaultMessagesSubscription( internal class DefaultMessages( private val roomId: String, - realtimeChannels: AblyRealtime.Channels, + private val realtimeChannels: AblyRealtime.Channels, private val chatApi: ChatApi, ) : Messages, ContributesToRoomLifecycleImpl(), ResolvedContributor { + override val featureName: String = "messages" + private var listeners: Map> = emptyMap() private var channelStateListener: ChannelStateListener @@ -239,8 +241,6 @@ internal class DefaultMessages( */ private val messagesChannelName = "$roomId::\$chat::\$chatMessages" - override val featureName: String = "messages" - override val channel = realtimeChannels.get(messagesChannelName, ChatChannelOptions()) override val contributor: ContributesToRoomLifecycle = this @@ -303,10 +303,6 @@ internal class DefaultMessages( override suspend fun send(params: SendMessageParams): Message = chatApi.sendMessage(roomId, params) - fun release() { - channel.off(channelStateListener) - } - /** * Associate deferred channel serial value with the current channel's serial * @@ -371,6 +367,10 @@ internal class DefaultMessages( } } } + + override fun release() { + realtimeChannels.release(channel.name) + } } /** diff --git a/chat-android/src/main/java/com/ably/chat/Occupancy.kt b/chat-android/src/main/java/com/ably/chat/Occupancy.kt index 4a3f567b..65fd7e94 100644 --- a/chat-android/src/main/java/com/ably/chat/Occupancy.kt +++ b/chat-android/src/main/java/com/ably/chat/Occupancy.kt @@ -80,4 +80,8 @@ internal class DefaultOccupancy( override suspend fun get(): OccupancyEvent { TODO("Not yet implemented") } + + override fun release() { + // No need to do anything, since it uses same channel as messages + } } diff --git a/chat-android/src/main/java/com/ably/chat/Presence.kt b/chat-android/src/main/java/com/ably/chat/Presence.kt index be9e4e7d..e3363c4a 100644 --- a/chat-android/src/main/java/com/ably/chat/Presence.kt +++ b/chat-android/src/main/java/com/ably/chat/Presence.kt @@ -167,4 +167,8 @@ internal class DefaultPresence( override fun subscribe(listener: Presence.Listener): Subscription { TODO("Not yet implemented") } + + override fun release() { + // No need to do anything, since it uses same channel as messages + } } diff --git a/chat-android/src/main/java/com/ably/chat/Room.kt b/chat-android/src/main/java/com/ably/chat/Room.kt index 5922e7cb..4930fc75 100644 --- a/chat-android/src/main/java/com/ably/chat/Room.kt +++ b/chat-android/src/main/java/com/ably/chat/Room.kt @@ -105,6 +105,11 @@ interface Room { * Detaches from the room to stop receiving events in realtime. */ suspend fun detach() + + /** + * Releases the room, underlying channels are removed from the core SDK to prevent leakage. + */ + suspend fun release() } internal class DefaultRoom( @@ -199,7 +204,7 @@ internal class DefaultRoom( reactions.channel.detachCoroutine() } - fun release() { - messages.release() + override suspend fun release() { + _lifecycleManager?.release() } } diff --git a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt index 08f94c38..e6762c74 100644 --- a/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt +++ b/chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt @@ -40,6 +40,11 @@ interface ContributesToRoomLifecycle : EmitsDiscontinuities, HandlesDiscontinuit * @returns The error that should be used when the feature fails to detach. */ val detachmentErrorCode: ErrorCodes + + /** + * Underlying Realtime feature channel is removed from the core SDK to prevent leakage. + */ + fun release() } abstract class ContributesToRoomLifecycleImpl : ContributesToRoomLifecycle { @@ -679,6 +684,9 @@ class RoomLifecycleManager( } } }.awaitAll() + _contributors.forEach { + it.contributor.release() + } _releaseInProgress = false _statusLifecycle.setStatus(RoomStatus.Released) } diff --git a/chat-android/src/main/java/com/ably/chat/RoomReactions.kt b/chat-android/src/main/java/com/ably/chat/RoomReactions.kt index 12cca2b1..72f6ab59 100644 --- a/chat-android/src/main/java/com/ably/chat/RoomReactions.kt +++ b/chat-android/src/main/java/com/ably/chat/RoomReactions.kt @@ -106,13 +106,14 @@ data class SendReactionParams( internal class DefaultRoomReactions( roomId: String, private val clientId: String, - realtimeChannels: AblyRealtime.Channels, + private val realtimeChannels: AblyRealtime.Channels, ) : RoomReactions, ContributesToRoomLifecycleImpl(), ResolvedContributor { + override val featureName = "reactions" + private val roomReactionsChannelName = "$roomId::\$chat::\$reactions" override val channel: AblyRealtimeChannel = realtimeChannels.get(roomReactionsChannelName, ChatChannelOptions()) - override val featureName = "reactions" override val contributor: ContributesToRoomLifecycle = this @@ -161,4 +162,8 @@ internal class DefaultRoomReactions( channel.subscribe(RoomReactionEventType.Reaction.eventName, messageListener) return Subscription { channel.unsubscribe(RoomReactionEventType.Reaction.eventName, messageListener) } } + + override fun release() { + realtimeChannels.release(channel.name) + } } diff --git a/chat-android/src/main/java/com/ably/chat/Rooms.kt b/chat-android/src/main/java/com/ably/chat/Rooms.kt index 0d99e047..a316e0e7 100644 --- a/chat-android/src/main/java/com/ably/chat/Rooms.kt +++ b/chat-android/src/main/java/com/ably/chat/Rooms.kt @@ -72,9 +72,7 @@ internal class DefaultRooms( } override suspend fun release(roomId: String) { - synchronized(this) { - val room = roomIdToRoom.remove(roomId) - room?.release() - } + val room = roomIdToRoom.remove(roomId) + room?.release() } } diff --git a/chat-android/src/main/java/com/ably/chat/Typing.kt b/chat-android/src/main/java/com/ably/chat/Typing.kt index 2f6fe85c..42dd2e94 100644 --- a/chat-android/src/main/java/com/ably/chat/Typing.kt +++ b/chat-android/src/main/java/com/ably/chat/Typing.kt @@ -107,4 +107,8 @@ internal class DefaultTyping( override suspend fun stop() { TODO("Not yet implemented") } + + override fun release() { + realtimeClient.channels.release(channel.name) + } }