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

Question: Observable stuck in case StackoverflowException thrown inside Task {}, is it expected? #1671

Open
GrigorievNick opened this issue Nov 16, 2022 · 7 comments

Comments

@GrigorievNick
Copy link

import monix.eval.Task
import monix.execution.Scheduler.Implicits.global
import monix.reactive.Consumer
import monix.reactive.Observable
import org.scalatest.FunSuite

import scala.concurrent.duration._

class MonixExceptionTest extends FunSuite {

  test("fatal") {
    Observable
      .fromIterable(0 to 10)
      .mapParallelUnordered(4)(i =>
        Task {
          try throw new StackOverflowError(s"$i")
          catch {
//            case x: Throwable => throw new RuntimeException(x)
            case x: Throwable => throw x
          }
          i
        }
      )
      .consumeWith(Consumer.foreach(println))
      .runSyncUnsafe()
  }

}

this code will be stuck forever.
But if we wrap Stackoverflow with RuntimeException, Observable will fail.
Do this expected behave?

@ghik
Copy link
Contributor

ghik commented Nov 17, 2022

Yes, this is because StackOverflowError is considered fatal and unrecoverable.

Traditionally, all Errors are considered fatal (as opposed to Exceptions). Scala has a slightly refined definition of what is fatal (see scala.util.control.NonFatal extractor) but StackOverflowError falls into both definitions so Monix will not try to catch and recover from it.

@GrigorievNick
Copy link
Author

OK, but still I expect to fail observable, rather then just silently stuck.
Or I miss understand something?

@ghik
Copy link
Contributor

ghik commented Nov 18, 2022

For an Observable to fail on StackOverflowError, it would have to catch it at some point. But fatal errors should never be caught. They should be allowed to crash the process.

Although I remember that at some point NonFatal did not recognize StackOverflowError as fatal so it may be somewhere in a gray area.

@GrigorievNick
Copy link
Author

GrigorievNick commented Nov 18, 2022

I am not sure that problem on Observable side, look like Task is never complete with fail or success - no matter.

@ghik
Copy link
Contributor

ghik commented Nov 18, 2022

Yes, but Task doesn't catch it for exactly the same reason.

@GrigorievNick
Copy link
Author

GrigorievNick commented Nov 18, 2022

OK, so what is way to deal with this issue: improve error handling on Monix side, or always wrapp code inside Task with try-catch, or add timeout to every Task?

@GrigorievNick
Copy link
Author

Do you happen to have any news on this?
Code must throw an exception and not be silently stuck even without logs.

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

No branches or pull requests

2 participants