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

Counter-intuitive behavior with interruptScope and IOLocals #2842

Open
armanbilge opened this issue Mar 9, 2022 · 2 comments
Open

Counter-intuitive behavior with interruptScope and IOLocals #2842

armanbilge opened this issue Mar 9, 2022 · 2 comments
Labels

Comments

@armanbilge
Copy link
Member

armanbilge commented Mar 9, 2022

Context: armanbilge/bayou#1 (comment)

//> using scala "3.1.1"
//> using lib "co.fs2::fs2-core:3.2.5"

import cats.effect._
import fs2._

object Bayou extends IOApp.Simple:
  // def run = s.compile.lastOrError.flatMap(IO.println) // List(2, 1)
  // def run = s.scope.compile.lastOrError.flatMap(IO.println) // List(2, 1)
  def run = s.interruptScope.compile.lastOrError.flatMap(IO.println) // List()

  def s = Stream.eval(IOLocal(List.empty[Int])).flatMap { local =>
    Stream.eval(local.update(1 :: _)).flatMap { _ =>
      Stream.eval(local.update(2 :: _)).flatMap { _ =>
        Stream.eval(local.get)
      }
    }
  }
@armanbilge armanbilge added the bug label Mar 9, 2022
@armanbilge
Copy link
Member Author

Thanks to Diego and Michael, we find our way to the eval on InterruptContext:

def eval[A](fa: F[A]): F[Either[InterruptionOutcome, A]] =
ref.get.flatMap {
case Some(outcome) => F.pure(Left(outcome))
case None =>
F.race(deferred.get, fa.attempt).map {
case Right(result) => result.leftMap(Outcome.Errored(_))
case Left(other) => Left(other)
}
}

Because the effect fa is raced, it runs on its own fiber which means any changes to local state are local to its own fiber and subsequently discarded.

There's some discussion of IOLocal's join semantics in typelevel/cats-effect#1393 (comment) but that is unlikely to change.

So this is seeming like a cantfix.

@armanbilge
Copy link
Member Author

I asked if we can designate a portion of the Stream as non-interruptible, and Michael thinks this should be possible to implement.

to implement uninterruptible regions of streams, we'd probably need to augment InScope constructor of pull to take a third state -- it currently takes a useInterruption: Boolean but would need something like interruption: Disabled | Enabled | Inherited

https://discord.com/channels/632277896739946517/839257183782436945/951095281619976272

The relevant region of code, which already describes the existing states.

def open(interruptible: Boolean): F[Either[Throwable, Scope[F]]] = {
/*
* Creates a context for a new scope.
*
* We need to differentiate between three states:
* The new scope is not interruptible -
* It should respect the interrupt of the current scope. But it should not
* close the listening on parent scope close when the new scope will close.
*
* The new scope is interruptible but this scope is not interruptible -
* This is a new interrupt root that can be only interrupted from within the new scope or its children scopes.
*
* The new scope is interruptible as well as this scope is interruptible -
* This is a new interrupt root that can be interrupted from within the new scope, its children scopes
* or as a result of interrupting this scope. But it should not propagate its own interruption to this scope.
*
*/

it seems what's missing is a way to be non-interruptible and ignore the interrupt of the parent scope

and i think that's as simple as saying when disabled is requested, fall in to the case None

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

Successfully merging a pull request may close this issue.

1 participant