Guard notifying outdated scheduler

[rorbech]

TODOs:

• Await that core fix is merged Change the C API around custom scheduler realm-core#7102

Closes #1543
Closes #1558
Closes #1561

[clementetb]

Looks great.

[clementetb]

Doesn't core wait until the jobs have been executed to release the scheduler?

Also, this guard does not cover 100% of the cases, the pointer might have been released just after accessing cancelled.

Because closing and job execution happens on the same thread there is no way that the Realm gets closed while executing the Job.

[clementetb]

Instead of guarding the SingleThreadDispatcherScheduler, we could cancel the coroutine scope just after closing the live Realm here.

Closing the coroutine there would cancel any pending core jobs that were posted.

[rorbech]

Instead of guarding the SingleThreadDispatcherScheduler, we could cancel the coroutine scope just after closing the live Realm here.

Closing the coroutine there would cancel any pending core jobs that were posted.

If we have a user supplied scheduler then we cannot close it. There could also potentially be multiple scheduler in the same scope so feels better to use the exact signal from the user-data-free-callback to guard this.

[clementetb]

Yes, we shouldn't be closing the main CoroutineContext after releasing the dispatcher.

The if-guard works, but depends on AtomicBoolean and requires to implement it on each platform. Coroutines have some structures to handle job cancelation gracefully.

For example instead of working with the main CoroutineContext we could have a specific CoroutineScope associated to the scheduler to track all the jobs that have been posted via the scheduler. Once the scheduler is deleted, we can cancel the scope, and thus all pending jobs.

[rorbech]

But coroutine scopes are not preemptive so you are not guaranteed that any running coroutine will be immediately aborted. But you are right the current guard is actually not good enough ... though it looks like some of the schedulers in core uses a similar flag so might be that there are some other guarantees in effect here 🤔

[nhachicha]

Nice fix 👍 it looks like io.realm.kotlin.test.darwin.CoroutineTests.dispatchBetweenThreads is failing for macOS on CI

[rorbech]

Nice fix 👍 it looks like io.realm.kotlin.test.darwin.CoroutineTests.dispatchBetweenThreads is failing for macOS on CI

The failing test is caused by trying to clean up the liveRealmContext in f3e4ce3#diff-291d3886fa0a21b98dc0ebd5ff0ce3846d78dab19a8d3ccffe6386899f0879a4. I have verified that it also happens on main if I try it out there so it has nothing to do with this PR, hence I therefore removed it from this PR to avoid stalling it.

[cmelchior]

Awesome 💯

[cmelchior]

Great catch 🙈, but any reason you are adding a timeout? The first upload doesn't have one

[rorbech]

I added it because it would highlight what action is actually not executing as expected instead of just timeout out on the recipient side. I just added if for the uploads that I inserted, but just didn't walk over the rest of the code.

[cmelchior]

Ok 👍