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

Consolidate and document order between cancellation handlers and continuation resume #415

Closed
qwwdfsad opened this issue Jul 2, 2018 · 4 comments
Labels

Comments

@qwwdfsad
Copy link
Collaborator

qwwdfsad commented Jul 2, 2018

Reproducer from #407 will fail with exception in coroutines machinery

@Test
fun testCancelProduce() = runTest {
    val source = Channel<Int>()
    val produced = produce<Int>(ctx, onCompletion = source.consumes()) {
        source.receive() // <- when exception is thrown from here, source is NOT empty for Unconfined
    }
  
    produced.cancel()
}

Behaviour will be different for Unconfined and coroutineContext as ctx.

It's because we do the following:

cont.cancel(e) 
->  dispatch() { cont.resumeWithException(e) }
->  for (handler in handler) handler.invoke(e)

We have data-race with small window between dispatched continuation and handlers invocation. Same race is 100% reproducible with Unconfined.

@qwwdfsad qwwdfsad added the bug label Jul 2, 2018
qwwdfsad added a commit that referenced this issue Jul 3, 2018
1) Invoke handlers before dispatching in CancellableContinuation to make behaviour timing-independent and Unconfined doesn't produce unexpected results
2) Invoke onCancellation -> handlers -> onCompletion in Job to make behaviour timing-independent

Fixes #415
@PaulWoitaschek
Copy link
Contributor

So this is fixed but not released yet?

@qwwdfsad
Copy link
Collaborator Author

Yes

@PaulWoitaschek
Copy link
Contributor

Can this please be released soon? About 0.7% of my sessions are crashing with this exception.

@qwwdfsad
Copy link
Collaborator Author

Presumably it will be released next week

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