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

Even though async is already cancelled, it may fail with other Exceptions. #675

Closed
yamasa opened this issue Oct 7, 2018 · 1 comment
Closed
Assignees
Labels

Comments

@yamasa
Copy link

yamasa commented Oct 7, 2018

I have a question about this test code.

class CancelBlockingCallTest {

    private fun someBlockingIO(): String {
        Thread.sleep(3000L)
        println("throwing IOException...")
        throw IOException()
    }

    @Test
    fun testWithContext() = runBlocking {
        val job = launch(Dispatchers.Default) {
            try {
                val result = withContext(Dispatchers.IO) {
                    someBlockingIO()
                }
                println("result: $result")
            } catch (e: IOException) {
                println("exception: $e")
            }
        }
        delay(1000L)
        job.cancel()
        job.join()
    }

    @Test
    fun testAsync() = runBlocking {
        val job = launch(Dispatchers.Default) {
            try {
                val deferred = async(Dispatchers.IO) {
                    someBlockingIO()
                }
                println("result: ${deferred.await()}")
            } catch (e: IOException) {
                println("exception: $e")
            }
        }
        delay(1000L)
        job.cancel()
        job.join()
    }

    @Test
    fun testCoroutineScopeAndAsync() = runBlocking {
        val job = launch(Dispatchers.Default) {
            try {
                coroutineScope {
                    val deferred = async(Dispatchers.IO) {
                        someBlockingIO()
                    }
                    println("result: ${deferred.await()}")
                }
            } catch (e: IOException) {
                println("exception: $e")
            }
        }
        delay(1000L)
        job.cancel()
        job.join()
    }
}

testWithContext() is a typical code that calls a blocking IO function.
In this test, the job is cancelled before returning from someBlockingIO(). As a result, the IOException is just ignored.

testAsync() is just using async instead of withContext. But, its result is different from testWithContext(). Even though the job was already cancelled, async and its parent are failed with IOException.

I think if async has already been cancelled, it should ignore any other Exceptions. What do you think?

@yamasa yamasa changed the title Even though async was already cancelled Even though async is already cancelled, it may fail with other Exceptions. Oct 7, 2018
@elizarov
Copy link
Contributor

elizarov commented Oct 8, 2018

Ignoring exceptions is a really, really bad idea for any serious production code. I actually think that ignoring exception in testWithContext case is actually a bug and it should behave more like async/await and coroutineScope.

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

No branches or pull requests

2 participants