but I suspect more bugs

@armanbilge why do you suspect more bugs?

It's because the same mistake was made in 4 places :) I think I fixed all of them now.

Is it my code? I remember going back and forth on this one and possibly mucking it up. Sorry.

Eh, blame the reviewer 😉

Anyway, thanks to @t3hnar for catching this, I'm about the invest the CE.js world pretty deep into syncStep.

syncStep is critical functional for projects in my company as well. Mostly because it is used for integration with akka/kafka/cassandra/java libraries, and things could not be done without it, as otherwise you would be either loosing order guarantees or blocking threads.

@t3hnar I'd be interested to see a specific example of your use-case. I wonder if the work in #2854 will be applicable to you? It provides an alternate mode for Dispatcher which guarantees ordering of effects.

Actually, syncStep was primarily introduced to help with JavaScript interop. In JVM-world, I think use-cases should be much more rare and Dispatcher is probably a better tool.

Any code where you have java listeners called in exact order that should start executing IO based code.
And ordering guaranteed are modelled inside of IO composition for instance via Ref (not crossing async boundary)

Also in cases where you cannot use IO.async - because you want to have IO[IO[A]] instead of classic IO[A] - outer IO is about java api being called, inner IO is about execution is completed.

def javaApi: JavaFuture[A] = ???
def scalaApi: IO[IO[A]] = {
  for {
  future <- IO { javaApi } // because this gives order guarantee during method call and does not cross async boundary
  } yield {
    future.javaFutureToIO
  }
}

Any code where you have java listeners called in exact order that should start executing IO based code.

This really sounds like exactly what #2854 was designed to fix.

Also in cases where you cannot use IO.async - because you want to have IO[IO[A]] instead of classic IO[A] - outer IO is about java api being called, inner IO is about execution is completed.

Fwiw, you might want to consider modeling this as Resource[IO, IO[A]] if javaApi involves any scarce resources. I definitely get what you're going at though. This type of pattern is annoying precisely because older-style APIs (such as in Java) often model these types of lifecycles implicitly ("kick off a running future and which you can ask for later") rather than explicitly (IO[IO[A]]).

This really sounds like exactly what #2854 was designed to fix.

already subscribed and looking forward

I have no intuition as to why this would cause doubled execution…

because we returned original io as IO and synchronous part of it as SyncIO
and executing one after another leads to double execution of synchronous part

Here's an example:

val io1 = IO(println(...))
val io2 = io1 *> IO.cede
val io3 = io2.void

If you run io3.syncStep.unsafeRunSync, roughly this happens:

1. We pattern match io3 @ IO.Map(io2, _ => ())
2. We interpret io2:
   1. We pattern match io2 @ IO.FlatMap(io1, _ => IO.cede)
   2. We interpret io1:
      1. We pattern match io1 @ IO.delay(println(...))
      2. We run it synchronously, and return Right(())
   3. We apply the flatMap f to () and get IO.cede.
   4. We interpret IO.cede, but can't, so we return Left(IO.cede)
3. We get Left(IO.cede), so we have to bail. Now, we have an interesting question: should we return the Left(io.void) with the io we've just been given, or should we return Left(io3) that we started the iteration with? The old code was doing the latter.

Hope that makes sense.