Fix various dispatcher issues #1611
Conversation
packages/test-base/src/jvmTest/kotlin/io/realm/kotlin/test/jvm/RealmTests.kt
Outdated
Show resolved
Hide resolved
...e/src/commonTest/kotlin/io/realm/kotlin/test/common/notifications/RealmNotificationsTests.kt
Show resolved
Hide resolved
| e.printStackTrace() | ||
| } | ||
| } catch (ex: IllegalStateException) { | ||
| // A work-around for https://github.com/realm/realm-kotlin/issues/1608 |
There was a problem hiding this comment.
I guess we have had some debate on which way to organize the dispatcher/scheduler relationship (#1559 and #1463 (review)), but this seems like a very unsafe way of guarding this. We already have infrastructure in place for being able to cancel the SingleThreadDispatcherScheduler, so seems way safer to expose the cancel to the upper layers, so that we follow the exact logic on when we expect this to happen (when we close the realm?). I guess that wouldn't work if people inject their own dispatcher and just close it, but we could document that they shouldn't close the dispatcher before closing the realm.
There was a problem hiding this comment.
I modified the implementation, so we now have two locks instead, one to control the dispatcher and one for the scheduler.
…when the scheduler is released.
…/RealmTests.kt Co-authored-by: Claus Rørbech <[email protected]>
|
Ready for a second round @rorbech |
| @@ -120,6 +120,10 @@ public class LiveRealmContext( | |||
| } | |||
|
|
|||
| override fun close() { | |||
| // Warning: It is important to release the scheduler before closing the | |||
There was a problem hiding this comment.
I guess this will work if we are guaranteed the user_data_free callback from core synchronously from realm_release(scheduler). It would probably be safer for future changes with a separate abstraction of the scheduler that would set dispatcherClosing before realm_release(scheduler) ... but this is way better that the previous implementation that was relying on the message from the coroutine-exception 🙈
Close #1608
See also Kotlin/kotlinx.coroutines#3993