-
Notifications
You must be signed in to change notification settings - Fork 529
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
Propagate Java thread interruption in Dispatcher#unsafeRunSync
#4167
Propagate Java thread interruption in Dispatcher#unsafeRunSync
#4167
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also make a similar change for IO#unsafeRunSync
.
std/jvm/src/main/scala/cats/effect/std/DispatcherPlatform.scala
Outdated
Show resolved
Hide resolved
tests/jvm/src/test/scala/cats/effect/std/DispatcherJVMSpec.scala
Outdated
Show resolved
Hide resolved
tests/jvm/src/test/scala/cats/effect/std/DispatcherJVMSpec.scala
Outdated
Show resolved
Hide resolved
tests/jvm/src/test/scala/cats/effect/std/DispatcherJVMSpec.scala
Outdated
Show resolved
Hide resolved
tests/jvm/src/test/scala/cats/effect/std/DispatcherJVMSpec.scala
Outdated
Show resolved
Hide resolved
I got a bit lost trying to comprehend the nuanced
|
Actually, maybe let's hold off on that change. I forgot that the specification for this method is a bit unusual. cats-effect/core/jvm/src/main/scala/cats/effect/IOPlatform.scala Lines 45 to 48 in 725a4cd
But if we did decide to move forward ... You can factor out a package-private version of cats-effect/core/shared/src/main/scala/cats/effect/IO.scala Lines 979 to 992 in 725a4cd
Then, you can use that for cancelation in cats-effect/core/jvm/src/main/scala/cats/effect/IOPlatform.scala Lines 79 to 85 in 725a4cd
|
Co-authored-by: Arman Bilge <armanbilge@gmail.com>
If only you are ok with the resulting discrepancy in sematics between |
Dispatcher#unsafeRunSync
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
Closes #4166.