Guidance on custom implementations of coroutine dispatchers, and usage of Delay

[martaciesielska]

I've been working on some code that uses a custom implementation of
CoroutineDispatcher for testing. Our test dispatcher uses fake time, and runs
everything on a single thread to ensure that tests are deterministic. It's not
appropriate for end-to-end testing but it works well with a lot of unit tests
(simplicity, speed, determinism). Due to the
inner workings of Kotlin coroutines,
we made our custom dispatcher implement the Delay interface to be able to
control scheduling, even though Delay is marked as
an internal API.

Additionally to the test dispatcher, we want to maintain a coroutine dispatcher
decorator that facilitates dispatch/suspension observability. This decorator
would look something like this:

internal class CoroutineDispatcherDecorator(
  private val delegate: CoroutineDispatcher,
  private val observer: Observer, // our interface for observing some events in the system
) : CoroutineDispatcher() {

  override fun dispatch(context: CoroutineContext, block: Runnable) {
    observer.onDispatch(context)
    delegate.dispatch(context) {
      try {
        block.run()
      } finally {
        observer.onSuspend(context)
      }
    }
  }
}

With this decorator, we run into another problem with Delay: if delegate
actually implements it, we hide it by using the decorator.

Given the above, I wanted to ask some questions:

1. Are there any plans to make Delay public or provide some other public API
   in its place? I've seen
   one issue from 2020
   where it was actually recommended to someone to implement Delay, is this
   advice up-to-date? I understand that if we use Delay, we run the risk of our
   code getting broken with some future release of Kotlin coroutines.

2. If not, are custom implementations of dispatchers discouraged? Neither
   CoroutineDispatcher nor ContinuationInterceptor are marked as internal so at
   first glance it seems like this is OK but not implementing Delay could lead to
   unexpected behaviour. Is there any guidance on this (I haven't found it in the
   documentation but maybe I've missed it)?

[dkhalanskyjb]

Why do you want to implement your custom CoroutineDispatcher for testing with fake time, when we already provide such a mechanism https://github.com/Kotlin/kotlinx.coroutines/tree/master/kotlinx-coroutines-test?

I think you raise an interesting point regarding cases when you want to decorate a dispatcher while keeping the Delay implementation. This is not my first time seeing people wrapping our dispatchers with something. Maybe we should provide some proper way to do just that while keeping the Delay implementation, something like class CoroutineDispatcherWrapper(wrapped: CoroutineDispatcher): CoroutineDispatcher(), Delay. Would this cover your use case?

[martaciesielska]

Why do you want to implement your custom CoroutineDispatcher for testing with fake time, when we already provide such a mechanism https://github.com/Kotlin/kotlinx.coroutines/tree/master/kotlinx-coroutines-test?

1. It's marked as experimental which is a bit of a deterrent (maybe not as strong as internal but still).

2. It doesn't give us the same amount of flexibility - apart from manipulating time, in some cases we also enforce certain rules as to which work items get priority for execution, to simulate race conditions that we know might occur in a live system.

3. On the flip side of point 2), having our own dispatcher also gives us an opportunity to remove complexity where we don't need it if we're not using particular coroutines' features, which can make for very easy debugging.

I think you raise an interesting point regarding cases when you want to decorate a dispatcher while keeping the Delay implementation. This is not my first time seeing people wrapping our dispatchers with something. Maybe we should provide some proper way to do just that while keeping the Delay implementation, something like class CoroutineDispatcherWrapper(wrapped: CoroutineDispatcher): CoroutineDispatcher(), Delay. Would this cover your use case?

That would definitely help with the decorator we want to put in our production code.

I'd still be keen to have a mechanism to implement my own dispatcher with scheduling support for testing, for the reasons listed above. Albeit not ideal, I'd be fine with using the wrapper if it allowed intercepting scheduling.

[martaciesielska]

Hi! Any updates on this? I’d really be keen to stop relying on the internal API for the decorator at least.

[qwwdfsad]

Not many updates, unfortunately.
For Delay to be useful, it requires a non-trivial rework and rethinking from the ground up, and it's slightly out of our scope right now.

Depending on internal API is okay-ish as long as it's done on the application level, not on the library one (e.g. Delay is not used by 3rd-party libraries that are shipped to be used by other clients that also depend on coroutines), Delay won't be broken on a single release basis and there will be a proper deprecation cycle

[bejibx]

Is it ok to wrap CoroutineDispatcher like this:

abstract class CoroutineDispatcherWrapper(
    private val dispatcher: CoroutineDispatcher,
) : CoroutineContext.Element {

    override val key: CoroutineContext.Key<*> = dispatcher.key

    override fun <R> fold(initial: R, operation: (R, CoroutineContext.Element) -> R): R {
        return operation(initial, dispatcher)
    }

    override fun <E : CoroutineContext.Element> get(key: CoroutineContext.Key<E>): E? {
        @Suppress("UNCHECKED_CAST")
        return if (this.key == key) dispatcher as E else null
    }

    override fun minusKey(key: CoroutineContext.Key<*>): CoroutineContext {
        return if (this.key == key) EmptyCoroutineContext else dispatcher
    }
}

looks like in this case wrapper is disappearing when added to CoroutineScope and this is exactly what I need. If you curios why am I doing this, it's for DI. In my team we are always using concrete classes when requesting dependencies. For example with RxJava:

class SomeRepository(
    private val workerScheduler: WorkerScheduler, 
    private val timerScheduler: TimerScheduler,
) {
    // ...
}

where WorkerScheduler and TimerScheduler are just our named wrappers around io.reactivex.Scheduler. I'm trying to do same thing with CoroutineDispatcher's and now I'm curios is it ok to wrap them like this or it can cause some unexpected problems?

[kenyee]

Maybe we should provide some proper way to do just that while keeping the Delay implementation, something like class CoroutineDispatcherWrapper(wrapped: CoroutineDispatcher): CoroutineDispatcher(), Delay.

FWIW, I think this would be helpful. Our use case is somewhat similar...we want to add Espresso idling resource wrappers for the coroutine dispatchers.