ZIO Core: Disconnect Children in Uninterruptible Race #3125
Conversation
|
Updated to just disconnect the children in |
|
@jdegoes Will not do anything without your approval! |
|
I think this problem should be solved by modifying: final class InterruptStatusRestore(private val flag: zio.InterruptStatus) extends AnyVal {
def apply[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] =
zio.interruptStatus(flag)
def force[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] =
if (flag == InterruptStatus.Uninterruptible) zio.uninterruptible.disconnect.interruptible
else zio.interruptStatus(flag)
}Then you can use: ZIO.uninterruptibleMask { interruptible =>
... interruptible.force(zio) ...
}Other comments inline. |
| val parentFiberId = descriptor.id | ||
| def maybeDisconnect[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] = | ||
| if (descriptor.interruptStatus.isInterruptible) zio | ||
| else zio.uninterruptible.disconnect.interruptible |
There was a problem hiding this comment.
Why zio.uninterruptible.disconnect.interruptible? It should be sufficient (and more performant) to do:
zio.disconnect.interruptible
There was a problem hiding this comment.
@jdegoes So that the underlying zio itself remains uninterruptible and can't be sent interrupts to - otherwise it could still be interrupted indirectly by supervision etc.
There was a problem hiding this comment.
When it's disconnected there's no way to interrupt it aside from having a reference to the fiber.
There was a problem hiding this comment.
@jdegoes
It will be interrupted inside disconnect:
final def disconnect: ZIO[R, E, A] =
ZIO.uninterruptibleMask(restore =>
for {
id <- ZIO.fiberId
fiber <- restore(self).forkDaemon
a <- restore(fiber.join).onInterrupt(fiber.interruptAs(id).forkDaemon)
} yield a
)With zio.disconnect.interruptible the following will print "interrupted uninterruptible never":
ZIO.sleep(5.seconds).race(ZIO.never.onInterrupt(putStrLn("interrupted uninterruptible never")))Example
Example 2: No message with uninterruptible
Because the right hand side will be interrupted in restore(fiber.join).onInterrupt(fiber.interruptAs(id).forkDaemon), i.e. omitting zio.uninterruptible makes all races interruptible as in RC17 – which I'm probably all for, but doesn't seem like what you intend.
Besides: all root fiber references are exposed publicly in Fiber.roots AND the interrupt flag can alter logic besides disallowing interrupts (it does so right here) – so it would still be much more sound to force the uninterruptible flag even if it didn't affect external interrupts (which it does)
There was a problem hiding this comment.
Ah right, sorry, I should know that. 😆
| @@ -3289,6 +3290,10 @@ object ZIO extends ZIOCompanionPlatformSpecific { | |||
| final class InterruptStatusRestore(private val flag: zio.InterruptStatus) extends AnyVal { | |||
| def apply[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] = | |||
| zio.interruptStatus(flag) | |||
|
|
|||
| def force[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] = | |||
There was a problem hiding this comment.
| def force[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] = | |
| /** | |
| * Returns a new effect that, if the parent region is uninterruptible, can be interrupted in the background | |
| * instantaneously. If the parent region is interruptible, then the effect can be interrupted normally, | |
| * in the foreground. | |
| */ | |
| def force[R, E, A](zio: ZIO[R, E, A]): ZIO[R, E, A] = |
Automatically disconnects children if
raceis performed in an uninterruptible region. I think there was a bug with the prior implementation ofdisconnect:The problem is that we call
restoreonfiber.join, which means if we are in an uninterruptible region joining the fiber is itself uninterruptible so so we suspend when we attempt to interrupt the join until the child fiber completes executing (possibly never). I think we want to change that line to befiber.join.interruptible. This leaves the underlying effect uninterruptible if it was in an uninterruptible region but allows us to continue with the effect in the background.