ably-labs / ably-chat-kotlin

Ably Chat SDK for Android
Apache License 2.0
1 stars 1 forks source link

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

Open sacOO7 opened 1 week ago

sacOO7 commented 1 week ago

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.
  2. 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
  3. 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.