Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async is not cancelled #320

Closed
SamKotlinFisher opened this issue Apr 10, 2018 · 8 comments
Closed

Async is not cancelled #320

SamKotlinFisher opened this issue Apr 10, 2018 · 8 comments

Comments

@SamKotlinFisher
Copy link

I have a function which load the image by click on button. If button is clicked twice (second click before async completion) it's needed to cancel old async and start new.
Code example:

private var deferred: Deferred<Bitmap>? = null
override fun onCreate(inState: Bundle?) {
        super.onCreate(inState)
        findViewById<View>(R.id.btn_action).setOnClickListener {
            if (deferred != null && deferred?.isActive == true) {
                Log.d("debug", "cancel")
                deferred?.cancel()
            }
            loadImage()
        }
    }

    private fun loadImage() = launch(UI) {
        Log.d("debug", "start")
        deferred = async {
           val conn = URL("http://elitefon.ru/pic/201503/2560x1600/elitefon.ru-38503.jpg").openConnection() as HttpURLConnection
            val bitmap = BitmapFactory.decodeStream(conn.inputStream)
            Log.d("debug", "end of async")
            return@async bitmap
        }
        deferred?.await()
        Log.d("debug", "loaded")
    }

In this case log will be something like this:

start
cancel
start
end of async
end of async
loaded

As we can see, in spite of cancel, "end of async" log is called twice.
Also, try/catch logging shown that CancelationException is throws in the launch(UI) block (not the async block).
Can I make it so that if I cancel async, the exception was thrown in the async block?

@jcornaz
Copy link
Contributor

jcornaz commented Apr 11, 2018

In coroutines cancellation is cooperative. The code in your async block is not actually cancellable.

You can make it actually cancellable by using yield or checking isActive.

Example:

deferred = async {
	val conn = URL("http://elitefon.ru/pic/201503/2560x1600/elitefon.ru-38503.jpg").openConnection() as HttpURLConnection
	yield() // throw CancellationException if the deferred is cancelled
	val bitmap = BitmapFactory.decodeStream(conn.inputStream)
	yield() // throw CancellationException if the deferred is cancelled
	Log.d("debug", "end of async")
	return@async bitmap
}

@SamKotlinFisher
Copy link
Author

@jcornaz, thanks for your answer, but this solution is not suitable for me. Most of the async time relates with BitmapFactory.decodeStream(conn.inputStream) and stream still read after cancelation. I want to interrupt stream reading after cancelation like cancel(true) of android.osAsyncTask.

@fvasco
Copy link
Contributor

fvasco commented Apr 11, 2018

Same issue here #173

Don't execute blocking code on common pool (it is really like the UI dispatcher).

Use a dedicated Executor and cancel (with interruption) the Future when necessary.

@SamKotlinFisher
Copy link
Author

@fvasco, I add CachedThreadPool as CoroutineDispatcher to async(CachedPool) like this:

val CachedPool = Executors.newCachedThreadPool().asCoroutineDispatcher()
async(CachedPool) {...}

But cancelation still not working what I expecting.
Сan you write code example?

@fvasco
Copy link
Contributor

fvasco commented Apr 11, 2018

Hi @SamKotlinFisher
cancelling coroutine does not interrupt the thread, so if you need to interrupt thread then you must use Future.cancel(...) or something similar.

See also: https://discuss.kotlinlang.org/t/calling-blocking-code-in-coroutines/2368/6

@SamKotlinFisher
Copy link
Author

@fvasco, thanks for your answer. I developed the similar solution:

fun <T> async (block: () -> T): Deferred <T?> {
     var thread: Thread? = null
     val deferred = async (CachedPool) {
         thread = Thread.currentThread ()
         return @async block.invoke ()
     }
     deferred.invokeOnCompletion (true, true) {
         if (deferred.isCancelled && thread! = null && thread? .isAlive == true) {
             thread? .interrupt ()
             thread = null
         }
     }
     return deferred
}

What do you think about this?

@fvasco
Copy link
Contributor

fvasco commented Apr 11, 2018

What do you think about this?

Use Future, you can use Guava integration

// global executor
val executor = MoreExecutors.listeningDecorator(Executors.newCachedThreadPool())

private var future: ListenableFuture<Bitmap>? = null

fun onCreate(inState: Bundle?) {
    future?.cancel(true)
    loadImage()
}

fun loadImage() {
    future = executor.submit<Bitmap> {
        val bitmap = ...
        return@submit bitmap
    }
    launch(UI) {
        future?.await()
        ...
    }
}

@SamKotlinFisher
Copy link
Author

@fvasco, thank you so much for your answer. I hope that this functionality will be able in kotlin-coroutine in a future as are described in #57.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants