[Proposal] New "reusable" scope

[DmitriyZaitsev]

Use case (the problem)

1. Some components, e.g. Android Fragments can be reused after it's "destruction".
   Unlike Activities that after onDestroy() become really dead and garbage collected, fragments have a different lifecycle and can stay physically alive after onDestroy()/onDetach(), for example cached for future use.

class MyFragment : Fragment(), CoroutineScope by MainScope() {
    // ...
    override fun onDestroy() {
        super.onDestroy()
        cancelCoroutineScope()
        // ...
    }
}

So, when we get an instance of such fragment that survived after destruction, we can no longer launch any coroutine in because the parent job is already canceled.

---

2. Those who use MVP pattern in their apps, often inject Presenters as singletons that live as long as the entire app lives.

class MyPresenter<V> {
    // ...
    fun takeView(v: V) { /*...*/ }
    fun dropView(v: V) { /*...*/ }
}

Naturally, presenters can be considered as coroutine scopes too.

class MyPresenter<MyView>: CoroutineScope by MainScope() {
    //...
    fun takeView(v: V) { /*...*/ } // enter scope
    fun dropView(v: V) { cancelCoroutineScope() } // exit scope
}

But in practice, a presenter can be used with different views, and its scope is likely determined by the lifecycle of the view attached to it - a lifetime in the interval between the presenter's takeView() and dropView().
Presenter drops view when a user rotates device or starts another activity etc. At the same time, the presenter should stop all running tasks. If it cancels coroutine scope once, we become unable to launch coroutines again.

---

3. The same goes for ViewModel, and I'm sure you can come up with your own examples.

Workarounds

1. Do not implement CoroutineScope itself.
   One can create the coroutine scope inside a fragment/presenter and manage it explicitly by hands.

class MyPresenter<MyView> {
    private var myScope: CoroutineScope? = null
    fun takeView(v: V) { buildCoroutineScope() } // enter scope
    fun dropView(v: V) { cancelCoroutineScope() } // exit scope
}

This might be error prone and requires writing some boilerplate code. But we all know that less code == less bugs.

---

2. Do not cancel the parent job, but it's children.

class MyPresenter<MyView>: CoroutineScope by MainScope() {
    //...
    fun takeView(v: V) { /*...*/ } // enter scope
    fun dropView(v: V) { cancelChildrenJobs() } // exit scope
}

The use of coroutineScope[Job]?.cancelChildren() instead of coroutineScope[Job]?.cancel() partially solves the issue: we get all jobs canceled except the parent.

Solution

Introduce new ReusableCoroutineScope.

internal class ReusableContextScope(
    context: CoroutineContext,
    private val newJob: () -> Job
) : CoroutineScope {
    private var reusableContext: CoroutineContext = context

    override val coroutineContext: CoroutineContext
        get() {
            if (reusableContext[Job]?.isCancelled == true) {
                reusableContext += newJob()
            }
            return reusableContext
        }
}

@Suppress("FunctionName")
fun ReusableCoroutineScope(
    context: CoroutineContext,
    newJob: () -> Job = { SupervisorJob() }
): CoroutineScope =
    ReusableContextScope(context, newJob)

One defines a newJob() function that's going to be used as a generator of new jobs in cases when the coroutineContext's job became canceled.
Also, inspired by #829, we can also add an alternative ReusableMainScope() factory function that will create a scope with Dispatchers.Main and re-inject new SupervisorJob to the context.

@Suppress("FunctionName")
fun ReusableMainScope(): CoroutineScope =
    ReusableContextScope(Dispatchers.Main) { SupervisorJob() }

Benefits

1. With this reusable scope, we can have a shiny nice short record for sensitive components.

class MyFragment : Fragment(), CoroutineScope by ReusableMainScope() {
    // ...
    override fun onDestroy() {
        super.onDestroy()
        cancelCoroutineScope()
    }
}

2. We don't care about managing jobs
3. We have safe code
4. Our coroutines still don't leak
5. If we need to reuse some canceled/destroyed component, we just take and use it like if it were a fresh new object.

@elizarov, @qwwdfsad, what do you think?

[fvasco]

I wish to expose my considerations:

Do not implement CoroutineScope itself

In my opinion this is not a workaround, if the Activity is reusable and the CororoutineScope not, maybe Activity !is CororoutineScope (is != has).

One defines a newJob() function that's going to be used as a generator of new jobs in cases when the coroutineContext's job became canceled.

We should does not assume that the Job is the only closeable resource, it is legal to put in the scope authentication details or a network connection, so I have to handle it on destroy (side note: cancel coroutine scope may not assume the job as only closeable resource).

ReusableContextScope

A ReusableContextScope is, the facto, a non cancellable CoroutineScope (similar to Workaround 2 solution), so it is always possible to launch a new coroutine in it (I can always assume in my Fragment isActive == true).
The workaround 1 provides a better scope life cycle handling, or we have to consider the CoroutineScope::restart method.

[DmitriyZaitsev]

Thanks @fvasco. Very correct reasoning!

In my opinion this is not a workaround [...] maybe Activity !is CororoutineScope (is != has).

Well, yes, but it might be. My thought is every object defines some kind of a scope, isn't it? So, if this object can run coroutines, then it must define a coroutine scope for them.
I think, it's much more handy not to write myScope.launch{...} but just launch{...}:
• semantics stays the same
• Kotlin lets us write a bit less code and avoid boilerplate

---

We should does not assume that the Job is the only closeable resource, it is legal to put in the scope authentication details or a network connection, so I have to handle it on destroy

You're right that all closeables must be handled.
Sorry for being unclear. I assumed, w/o breaking the generality, that we don't have other closeable resources.
My topic originally is only about closing the coroutine scope.

---

A ReusableContextScope is, the facto, a non cancellable CoroutineScope (similar to Workaround 2 solution)

Please let me disagree here. It IS cancellable in the real sense. As soon as it's canceled, all this job's children are cancelled in this case, too.

I can always assume in my Fragment isActive == true.

Generally speaking, you can't assume that. This depends on whether the job was running (was already started and has not completed nor was cancelled yet).
However, after the cancellation of the original Job, you just get the new one and can be sure that isCanceled == false. But the trick is you don't touch the coroutineContext until you need it, so the old job stays canceled, and the new job won't be created and injected unnecessarily.

[fvasco]

it's much more handy not to write myScope.launch{...} but just launch{...}

Yes, I agree, but we have (unfortunately) apply the same rule to isActive instead of myScope.isActive.
Moreover CoroutineScope does not define only a coroutineContext, but it contains a lot of method and Fragment inherits them, this can become confusing.

My topic originally is only about closing the coroutine scope.

We should take care to reopen a valid scope (ie: without reusing some authentication credential), creating a Job (and closing the old one) may not the only needed action.

But the trick is you don't touch the coroutineContext until you need it

It is a trick

        val myFragment = MyFragment()
        myFragment.cancelCoroutineScope()
        println("coroutineContext=${myFragment.coroutineContext}")

debugger/logger sometimes invoke toString()

[DmitriyZaitsev]

we have (unfortunately) apply the same rule to isActive instead of myScope.isActive.

I'm a bit confused here. Whose this isActive?

Moreover CoroutineScope does not define only a coroutineContext, but it contains a lot of method and Fragment inherits them, this can become confusing.

Do you mean it inherits all the extensions to CoroutineScope that use coroutineContext? Yes, including isActive. Indeed, it will always return true. Good question: is it a bug or a feature? On the one hand, this is really confusing. On the other - the usage of ReusableScope is explicit, and behavior shall be documented and therefore expected.
The problem may be that in the initial proposal we can't distinguish reusable scope from non-reusable (original). As an option, reusable scopes can implement some empty marker interface or/and have an extension property isReusable or/and it's context may have some other info about it (e.g. extra element).

We should take care to reopen a valid scope (ie: without reusing some authentication credential), creating a Job (and closing the old one) may not the only needed action.

Do you mean that cancelCoroutineScope() should be a member function of ReusableScope interface and all inheritors should not only cancel the job but also release other resources?
I consider cancelCoroutineScope() in the example as this function introduced in #866
I think, taking care of a stuff like reusing credentials does not apply to the original issue. Fragment/Presenter/ViewModel can do this in its corresponding method.

class MyPresenter<MyView>: CoroutineScope by ReusableCoroutineScope() {
    //...
    fun takeView(v: V) { /* do some IO stuff */ } // enter scope
    fun dropView(v: V) {
        cancelCoroutineScope()
        /* release other resources */
    } // exit scope
}

It is a trick [...] debugger/logger sometimes invoke toString()

Yes, in this case the job will be recreated, but how bad is this if it's empty, very cheap and does nothing?

[DmitriyZaitsev]

Hi, @fvasco I had some time to reconsider the initial idea and came up to the following changes.

1. if the scope is reusable, it should have its own interface
2. if the job inside was recreated at least once, we should know about it
3. since we provide a factory function for the job, we should be able to invoke it
4. if we have an instance of the CoroutineScope, we should be able to check if it's reusable

So, my original vision evolved into the following:

public interface ReusableCoroutineScope : CoroutineScope { // (1)
    public val isRestarted: Boolean // (2)
    public fun restartCoroutineContext() // (3)
}

public val CoroutineScope.isReusable: Boolean get() = this is ReusableCoroutineScope // (4)

internal class ReusableContextScope(
    context: CoroutineContext,
    private val newJob: () -> Job
) : ReusableCoroutineScope {
    private var _isRestarted = false
    private var _reusableContext: CoroutineContext = context

    override val isRestarted: Boolean get() = _isRestarted

    override val coroutineContext: CoroutineContext
        get() {
            if (_reusableContext[Job]!!.isCancelled) {
                restartCoroutineContext()
            }
            return _reusableContext
        }

    override fun restartCoroutineContext() {
        _reusableContext[Job]!!.cancel()
        _reusableContext += newJob()
        _isRestarted = true
    }
}

[fvasco]

Hi @DmitriyZaitsev
I try to response to your requests

I'm a bit confused here. Whose this isActive?

It is CoroutineScope::isActive, if myFragment isCoroutineScope so both myFragment.cancelCoroutineScope() and myFragment.isActive are valid.

The problem may be that in the initial proposal we can't distinguish reusable scope from non-reusable

I consider this a secondary problem.

Yes, in this case the job will be recreated, but how bad is this if it's empty, very cheap and does nothing?

This can be tricky to understand, especially if you are looking for a bug.

if the job inside was recreated at least once, we should know about it

Why this is useful?

since we provide a factory function for the job, we should be able to invoke it

Is the close method enough?

Finally I consider preferable the "workarounds" than your "solution".

[DmitriyZaitsev]

Hi @fvasco ,

It is CoroutineScope::isActive, if myFragment isCoroutineScope so both myFragment.cancelCoroutineScope() and myFragment.isActive are valid.

If getter does not recreate the job, myFragment.isActive will be equal to false after cancellation.
Let's separate then restart() from the coroutineContext getter so that the "reusable/restartable" scope would behave exactly the same as the original scope until we call restart() explicitly.
Then your concern is going to be satisfied as "tricky and unexpected" behavior disappears.

internal class RestartableContextScope(
    context: CoroutineContext,
    private val newJob: () -> Job
) : RestartableCoroutineScope {
    private var _isRestarted = false
    private var _coroutineContext: CoroutineContext = context

    override val isRestarted: Boolean get() = _isRestarted

    override val coroutineContext: CoroutineContext get() = _coroutineContext

    override fun restart() {
        _coroutineContext[Job]!!.cancel()
        _coroutineContext += newJob()
        _isRestarted = true
    }
}

This can be tricky to understand, especially if you are looking for a bug.

With the fix above, we don't have a bug.

Why this is useful?

This is useful to know whether the scope already been canceled and restarted.

preferable the "workarounds" than your "solution".

"solution" is the "workaround" encapsulated in the library.

Finally, we can have

class MyObject: RestartableCoroutineScope by RestartableMainScope() {
    fun onEnter() {
        if (!isActive) restart()
        // do some stuff, launch coroutines, etc
    }

    fun onExit() { cancelCoroutineScope() }
    // ...
}

or

class MyObject {
    private val scope = RestartableMainScope()
    fun onEnter() {
        if (!scope.isActive) scope.restart()
        // do some stuff, launch coroutines, etc
    }

    fun onExit() { scope.cancelCoroutineScope() }
    // ...
}

[plastiv]

I think we should consolidate the effort around #760. Proposed there syntax:

class MainActivity : AppCompatActivity() {
     override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        lifecycle.coroutineScope.launch {
            someSuspendFunction()
            someOtherSuspendFunction()
            someCancellableSuspendFunction()
        }
    }
}

• removes the need to understand difference activity.onDestroy vs fragment.onDestroy (https://developer.android.com/topic/libraries/architecture/lifecycle)
• We don't care about managing jobs ✅
• We have safe code ✅
• Our coroutines still don't leak ✅
• If we need to reuse some canceled/destroyed component, we just take and use it like if it were a fresh new object. ✅

[DmitriyZaitsev]

@plastiv that's different. #760 is about androidX Lifecycle aware components.

[qwwdfsad]

I don't think that reusable scope is a good fit for kotlinx.coroutines library:

1. Reusability is always a source of heisenbugs which occur in random places due to concurrent nature of the reusability. And it is not something that can be fully validated from within a reusable object itself. E.g. even the proposed PR contains multiple concurrency-related bugs.
2. "Performant" primitives are always acting like a flag "use me", especially when they come from the core library and we definitely don't want to encourage its usage.
3. Scope mutation is not a practice we'd recommend to use (it is trickier than it seems)
4. The proposed solution is a very project/architecture-specific and requires a lot of changes to have a clear semantics and invariants (e.g. now cancel cancels all children, then restart mutates context and cancelling children from a previous use can observe it).
5. An architecture-specific solution can be implemented outside of the library as it does not use any internal API.

This change is not worth it and carries more risks than value.

[DmitriyZaitsev]

@qwwdfsad fair enough. thanks for your comment!