Solvedkotlinx.coroutines Lifecycle handling

I was trying to use coroutine Jobs for Android lifecycle handling together with the UI dispatcher, but I've stumbled upon an interesting issue where basically the following code can cause a crash:

var job: Job? = null

fun foo() {
  job = launch(UI) {
    val bar = download().await()
    // Cancel could have been called now, but the following code is posted to a Handler and executed anyway
    performSomeWorkOnTheUI(bar) // Causes a crash after onPause
  }
}

override fun onPause() {
  job?.cancel()
}

A more elaborate example that consistently crashes due to the IllegalStateException thrown in onResume can be found here. The repo contains a complete executable project.

29 Answers

βœ”οΈAccepted Answer

Is there any problem to allow the code to be excecuted on the main thread after onDestroy per se?

Actually I believe this issue isn't limited to the main thread or even onDestroy. I'd say no matter what thread, it is at least unintuitive that a coroutine can resume execution after the corresponding job has been cancelled.

The problem, as I see it, is with the code that tries to use that state that was already destroyed in an implementation of onDestroy.

First, I think we need to to define what a resumption is. Is it the process of posting coroutine code to a queue or is it the actual start of execution? I'd argue for the latter definition since it doesn't depend on the internals of the dispatcher and thus may be what users intuitively think when they don't specifically observe the behavior of the coroutine library, which isn't very easy to grasp.

Any program that assumes that code following a suspension point of call to a cancellable function will not be executed after the job is cancelled even on the same thread is broken right now. In the following example the call to bar() will cause a crash due to state modification that happened before resumption, even though it happened on the same thread and the job was cancelled already. If I didn't know better, I wouldn't expect this to happen since I'd expect bar not to run if it didn't yet, setting didBarRun = true in the process.

coroutine

I am not yet convienced that the idea of directly modifing UI state from the coroutine is a sound arhitectural pattern that we shall endorce by providing the corresponding utility functions.

Are you sure people won't do this anyway even without proper library support? It compiles and seemingly works, except it will cause hard-to-debug crashes every once in a while, which will frustrate developers. I think the best you can do is provide appropriate tools to let them produce correct, though potentially architecturally unsound code.

Having given the above comment, I must stress that this is still an issue. It has to be solved.

Thanks for the acknowledgement and this discussion! πŸ™‚

Other Answers:

Thanks, I'll give that gist a try later today!

Asynchronous actions (coroutines) that are bound to UI lifecycle is a design smell, if I may say so.

I disagree. In the commonly used MVP architecture, binding asynchronous actions to the UI lifecycle is frequently done. For example, take a look at the CountriesPresenter in this example of the popular Mosby library. It fetches something from the network and then displays it in the UI, which is fine from a UX perspective if a progress indicator was shown before. It's also fine from a lifecycle POV since it checks isViewAttached() before accessing the UI in the callbacks.

This could be similarly achieved using RxJava, but without having to check isViewAttached():

var subscription: Subscription? = null
fun foo() {
  subscription = downloader.download()
                            .subscribeOn(Schedulers.io())
                            .observeOn(AndroidSchedulers.mainThread())
                            .subscribe { downloaded -> ... }
}
fun onPause() {
  subscription?.cancel()
}

I may be biased, but I've seen this pattern in a lot of apps already. Replacing it with the snippet from my initial post here seemed natural, but currently does not work as expected.

MVVM is an architecture that might work better with the current coroutine implementations in this library, but that I don't think architectures like MVP with asynchronous actions are a design smell. I'd argue that this library should ideally work well with all architectures.

E.g, when screen is rotated, then views should be destroyed and rebuilt, but all the models should continue to operate and to execute their asynchronous activities.

As mentioned before this doesn't apply when the application is finishing / shutting down. In that case, we're forced to either check isViewAttached() (or something similar, but after every suspending call, which is very error-prone) or have a reliable way of cancelling Jobs.

I hear your concern about channels, but I'm not familiar enough with the internals of this library to think of a solution. Thanks already for taking the time to investigate this!

I've played with this problem a bit and found the way to add the required cancellation behavior with a custom continuation interceptor. See this gist: https://gist.github.com/ilya-g/314c50369d87e0740e976818d2a09d3f

@elizarov Maybe you are right about not canceling it to present on resume, but, what if I want to:

override fun onPause() {
    if (isFinishing()) {
        job?.cancel()
    }
}

Then would be nice that using cancel really works, because our Activity no longer will be back and we know it. There are some checks that could be handled by the coroutine. metalabdesign/AsyncAwait, for example, do some checks and have different execution options, one safe that do some checks, another one that leave it do the developer.

@elizarov My bet, it doesn't really need to know. I know if we already downloaded, we should cache it and so on, but if my activity is finishing, maybe, trying to save it to cache can actually be harmful, what if saving to cache leads to a corrupt state? And actually, if I am calling cancel directly, I would expect it try to cancel as soon it can. If cancelling will stop me from saving to cache, then its my error as developer, not the library fault.

Android has the AsyncTask class, doing the same sample, using it, we got:

   @Override
   protected String doInBackground(String... params) {
      return download();
   }
   
   @Override
   protected void onPostExecute(T result) {
      super.onPostExecute(result);
      saveDownloadedDataToCache(result);
      performSomeWorkOnTheUI(result);
   }

onPostExecute(_:) method documentations states:

Runs on the UI thread after doInBackground(Params...). The specified result is the value returned by doInBackground(Params...).
This method won't be invoked if the task was cancelled.

As you can see, no cache either in this sample, but there are more callbacks in an AsyncTask, even one for cancelled tasks:

    @Override
    protected void onCancelled(@Nullable T result) {
      if (result != null) {
         saveDownloadedDataToCache(result);
      }
    }

Maybe some onCancelled, onError, finally handler can be used for it? Maybe another kind of flow control? metalabdesign/AsyncAwait, again, as example, had some experiments on it, like .awaitWithProgress(_:) that somehow can handle progress, or an chainable launch(_:) with onError and finally.

So trying to imagine something, could be:

  job = launch(UI) {
    val bar = download().await(finally={
      saveDownloadedDataToCache(it)
    })
    performSomeWorkOnTheUI(bar)
  }

OR

  job = launch(UI) {
    download().await().also {
      performSomeWorkOnTheUI(it)
    }
  }.finally {
     it?.let { saveDownloadedDataToCache(it) }
  }

I was looking through the coroutines guide, and there are some nice samples like the Run non-cancellable-block, maybe we could create different kinds of context like NonCancellable but for other uses, like for Updating UI, Data Flow, and so on.

  job = launch(UI) {
    val bar = download().await() // suspending call, and this can be made `NonCancellable`?
    saveDownloadedDataToCache(bar) 
    run(UISafe) { // UI call, cancellable-on-beginning, with safety-checks.
        performSomeWorkOnTheUI(bar)
    }
  }

Yet another option, maybe is too suggest, that for UI updates, we should have a suspending function, that somehow can use the UI Pool, and we can call it by, for example, .awaitOnUI().

  job = launch(UI) {
    val bar = download().await() // suspending call, and this can be made `NonCancellable`?
    saveDownloadedDataToCache(bar) 
    performSomeWorkOnTheUI(bar).awaitOnUI() // UI 'suspending' call, cancellable-on-beginning, with safety-checks.
  }

OR

  job = launch(UI) {
    val bar = download().await() // suspending call, and this can be made `NonCancellable`?
    saveDownloadedDataToCache(bar) 
    performSomeWorkOnTheUI(bar).await(UI) // UI 'suspending' call, cancellable-on-beginning, with safety-checks.
  }

Or yet, leave this one as it is, call it as Working-As-Intended, and add some AsyncTask wrapper, or just some implementation that can be chainable with an AsyncTask-like flow:

  launch(Android) {
    download().await().also {
      saveDownloadedDataToCache(it) 
      performSomeWorkOnTheUI(it)
    }
  }.onCancelled { 
     it?.let { saveDownloadedDataToCache(it) }
  }

Of course, maybe I am going too deep, maybe you are right, and we should create this ourselves for the projects we are working on, doing so in a generic way can be really tricky and we don't know how it will be used, but if any suggestion I made feel useful or interesting to you, maybe we can look further?

PS: Its late, I am tired, sorry if any inconsistency or redundancy in my text... Thank you for your time.

Related Issues:

82
kotlinx.coroutines Introduce StateFlow
@erikc5000 @igorwojda Indeed the naming for constructor functions is quite a controversial issue her...
65
kotlinx.coroutines Compilation error on the androidTest configuration after updating to 1.3.6
I found 2 workarounds: Exclude the duplicated files: Exclude the kotlinx-coroutines-debug dependency...
61
kotlinx.coroutines Flow.shareIn and stateIn operators
πŸ“£ There is an important question on the design of sharing operators we need community help with ...
37
kotlinx.coroutines Default dispatcher and UI dispatcher support for iOS
@kamerok below is the implementation I'm currently using Updated for coroutines 1.0 and implemented ...
24
kotlinx.coroutines Replacing Java Timer with Kotlin Coroutine Timer
You are welcome to use your startCoroutineTimer solution but we don't plan to a function like startC...
24
kotlinx.coroutines Help newbies to handle exceptions in coroutines
You can switch to async instead of launch and use await instead of join This way exceptions will per...
18
kotlinx.coroutines Flow.collects receives items after it was cancelled on a single thread
I have replaced every single collect with a safeCollect function in my project: It would be great if...
17
kotlinx.coroutines Provide abstraction for cold streams
I question the very need of Single/Solo The typealias digression was just to demonstrate what this c...
14
kotlinx.coroutines Support runBlocking for UI Tests
It is already reverted in develop branch Will be part of the next build (tentatively 0.24.1). ...
13
kotlinx.coroutines [question] Is this "IllegalStateException: This job has not completed yet" while using runBlockingTest normal?
I replaced runBlockingTest on runBlocking and it helped. Why when executing the following test: The ...
7
kotlinx.coroutines Lifecycle handling
Is there any problem to allow the code to be excecuted on the main thread after onDestroy per se? Ac...
6
kotlinx.coroutines BroadcastChannel.asFlow().onStart(...) is invoked before the subscription is opened
UNDISPATCHED is predictable: Prints: 132 In the docs we have this example of onStart(...): Not just ...
4
kotlinx.coroutines java.lang.NoSuchMethodError: kotlinx.coroutines.SupervisorKt.SupervisorJob
Does your version of kotlinx-coroutines-test matches with the version of kotlinx-coroutines-core? ...
96
fastapi WARNING: Unsupported upgrade request.
This error is not part of the FastAPI codebase When attempting to run this (using UviCorn) it starts...
84
actix web actix-web 1.0
1.0.0-rc is released next release is 1.0 I am planing to move 0.7 to separate branch and move 1.0 to...
64
fastapi [QUESTION] How to bridge Pydantic models with SQLAlchemy?
I just finished integrating Pydantic ORM mode into FastAPI it is released as version 0.30.0 πŸŽ‰ The n...
52
fastapi [QUESTION] How to send 204 response?
Instead of returning None and instead of injecting the response just return a newly created response...
51
swoole src Compile Error on Mac Big Sur with PHP 8
I can get it work by manually symlink the required file. Please answer these questions before submit...
51
swoole src Swoole's admin interface hot-loads code from a third-party server ?
At this stage all the member's releases have to use matyhtf's PECL account which is ridiculous for a...
42
fastapi OpenAPI UI not working properly when using automatic swagger-ui CDN (swagger-ui-3.30.1)
Thanks for reporting it and for all the discussion here everyone! πŸš€ β˜• Indeed it's a bug in Swagger ...
38
aiohttp "RuntimeError: Event loop is closed" when ProactorEventLoop is used
I found another solution for this problem if some still having issues with it This involves directly...
34
RxGo RxGo v2
Hey My opinion about what should be part of RxGo v2 General Iterable should be moved to an interface...
34
fastapi [QUESTION] Is this the correct way to save an uploaded file ?
@classywhetten FastAPI has almost no custom logic related to UploadFile -- most of it is coming from...
34
fastapi [QUESTION] Storing object instances in the app context
@ebarlas you're 100% right Description In Flask ...
34
tortoise orm Migrations
Hey guys I'm excited to announce that now we have a migrate tool written by pure python and just for...
32
You Dont Know JS let hoisting?
Hoisting is not a real thing It's a made up concept Hi thanks for taking the time to write such a gr...
30
fastapi [QUESTION] aiohttp integration best practice
That is one way if you want create a new session for every request You can also use a singleton appr...
26
fastapi logs with FastAPI and Uvicorn
Doing : is exactly what I was looking for ! Thank you dbanty. Hello Thanks for FastAPI easy to use i...
22
fastapi [QUESTION] Client Credentials Flow openAPI UI
I think I found the solution for others looking to implement the code - tiangolo has already enabled...
21
react query How to use useInfiniteQuery with custom props
The issue here is that you are not mapping up your query key to your query function's arguments prop...
21
fastapi FastAPI 0.65.2 POST request fails with "value is not a valid dict" when using the Requests library; 0.65.1 works (with a caveat)
Can confirm this still happens! We solved it by adding a -H Content-Type: application/json to the cu...
19
fastapi [QUESTION] Using pydantic models for GET request query params? Currently not possible, have to use dataclasses or normal classes.
@LasseGravesen You would do it like this: Check the docs here: https://fastapi.tiangolo.com/tutorial...
18
aiohttp ssl.SSLError: [SSL: KRB5_S_INIT] application data after close notify (_ssl.c:2605)
For those looking for a work-around to at least silence these exceptions: the traceback seen is outp...
18
fastapi [QUESTION] about threads issue with fastapi.
Hello Hi I have a question about the threads issue with fastapi ...
17
ava Allow tests files to have any extension (i.e. .jsx, .ts)
I think what @sindresorhus is getting at is the following With the latest release(0.13) and the abil...
17
ava Only exclude helpers directory when inside a test directory
I feel like we're talking past each other Any chance the exclude rule for helpers can be relaxed so ...
17
ava Flow type definition
Okay so I was planning on just playing around with it but you nerd sniped me and I ended up doing th...
16
react query Array of queries hook
@tannerlinsley yeah Problem: I have a use case were a component would ideally consume a dynamic numb...
15
fastapi [FEATURE] support for rate-limit
I've taken a stab at adapting flask-limiter to starlette and FastAPI Is your feature request related...
14
react query Unable to type useQueries options or results without casting
Hey @matthewdavidrodgers! As @TkDodo has already indicated there's a PR open which looks to improve ...
14
fastapi [QUESTION] How can I serve static files (html, js) easily?
Hi in case a solution is still needed though the issue is closed Description How can I serve static ...
14
fastapi Using UploadFile and Pydantic model in one request
Oups sorry I forgot I made custom validator to transform str to json for Model: ...
13
fastapi How can I pass the configuration in the app?
Is the example I posted above not clear enough? Without going into all the nuances of everything my ...
13
vibe.d undefined reference to methods in ssl
Also try to install libssl1.0-dev - this solves same problem for me Good day I'm trying to generate ...
12
You Dont Know JS Does const declaration is subject to the Variable Hoisting?
This example isn't actually about hoisting at all You don't call foo() until after you've declared a...
12
react query Thoughts on mutate function not handling rejected promises
I came to this issue because I was following the docs and my try/catch wasn't catching even though m...
12
fastapi [BUG] openapi.json fails to be generated for nested models
oups sorry I think your mistake is putting response_model=SimilarProducts in the wrong spot it's in ...
11
opencv4nodejs fatal error: 'tesseract/baseapi.h' file not found
@GiulioPettenuzzo Let's say you receive output like this from your shell: The non-ideal ...
11
pyrogram Pyrogram v1.2.9 auth not working. [406 Not Acceptable]: [406 UPDATE_APP_TO_LOGIN]
This issue has been fixed You can upgrade with the usual command pip install -U pyrogram. ...
11
fastapi Debug Logging (Maybe just a n00b issue)
ok so basically I'm using this in a applog package you use it wherever your entrypint is this way sh...