Skip to content
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

[ECO-4953][CHA-RL7] RoomLifecycle : Sequential Coroutinescope using select #31

Draft
wants to merge 3 commits into
base: feature/room-ATTACH
Choose a base branch
from

Conversation

sacOO7
Copy link

@sacOO7 sacOO7 commented Oct 22, 2024

  • Implemented RoomLifecycle Atomic Operations with a combination of Sequential Coroutinescope and Select

Few concerns

  1. On init, runs subscribeToJobEvents method which starts a while loop that should run indefinitely, even after room release.
  • This is to make sure attach/detach throws exceptions even when room is released.
  • Check CHA-RL1b, CHA-RL1c, CHA-RL2b, CHA-RL2c, CHA-RL2d.
  • This also means all available channels can't be closed since we are waiting for processing events, this can lead to memory leaks for closed rooms.
  • To avoid this, we have to add an extra locking mechanism which should be ideally avoided.
  1. Unfortunately implemenation has become more complicated than we thought because we also need to return the result of the async operation to the calling method i.e. attach, detach, release etc
  2. Implementation doesn't sync with chat-js, which we have maintained till now. We need to introduce additional interfaces. If time comes, it will be a bit difficult to correlate both codebases.

I am not biased towards any kind of implementation, but rather toward simplicity, readability, and usability and how better it aligns with the spec.

…cope-using-select

# Conflicts:
#	chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt
@sacOO7 sacOO7 changed the title RoomLifecycle : Sequential Coroutinescope using select [ECO-4953][CHA-RL7] RoomLifecycle : Sequential Coroutinescope using select Oct 22, 2024
@sacOO7 sacOO7 requested a review from ttypic October 23, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant