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

Implement optional thread interrupt on coroutine cancellation (#57) #1922

Closed
wants to merge 1 commit into from

Conversation

jxdabc
Copy link
Contributor

@jxdabc jxdabc commented Apr 19, 2020

See issue #57 for details

Signed-off-by: Trol jiaoxiaodong@xiaomi.com

)

See issue Kotlin#57 for details

Signed-off-by: Trol <jiaoxiaodong@xiaomi.com>
private fun initThread() {
val thread = Thread.currentThread()
state.loop { s ->
when {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps when (s) {?

override fun invoke(cause: Throwable?) {
state.loop { s ->
when {
s === Init || (s is Working && s.thread === null) -> {
Copy link

@taralx taralx Apr 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you swap initThread and initInvokeOnCancel then s.thread === null can't happen, right?

Copy link
Contributor Author

@jxdabc jxdabc Apr 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems a great idea, I will check it

@qwwdfsad qwwdfsad self-requested a review April 20, 2020 13:35
Copy link
Member

@qwwdfsad qwwdfsad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job!
Unfortunately, we are not ready to accept this PR. Changes are intrusive, with a lot of failure modes and corner cases, while the problem it is trying to solve is pretty marginal (especially when you know that a vast majority of Java code doesn't handle interruptions properly). We decided it's not worth to maintain it in a long term, sorry.

@jxdabc
Copy link
Contributor Author

jxdabc commented Apr 21, 2020

@qwwdfsad
This feature is of great use at least on Android as I could see. Please see ceses in #57 .
There is still another non-intrusive approach to this feature we are currently using, though not as easy to use as this one. I'll post another PR including that approach. Hope that would help.

jxdabc pushed a commit to jxdabc/kotlinx.coroutines that referenced this pull request Apr 22, 2020
…ellation (Kotlin#57)

This is implementation of issue Kotlin#57 and non-intrusive variant of Kotlin#1922

Signed-off-by: Trol <jiaoxiaodong@xiaomi.com>
@jxdabc
Copy link
Contributor Author

jxdabc commented Apr 22, 2020

@qwwdfsad
Hoping you'd have a look at:

#1934
(non-intrusive) Implement optional thread interrupt on coroutine cancellation (#57)

@elizarov
Copy link
Contributor

Superseded by #1934

@elizarov elizarov closed this Apr 23, 2020
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

Successfully merging this pull request may close these issues.

None yet

4 participants