Imported From: https://issues.scala-lang.org/browse/SI-9554?orig=1
Reporter: Jani Räty (jraty)
Affected Versions: 2.11.7
Attachments:

• bug.scala (created on Nov 11, 2015 9:01:16 PM UTC, 703 bytes)

This bug does not occur if a Promise is completed with a Failure containing a fatal Throwable; only if a fatal Throwable is thrown during execution of a Future's body or callback.

Looking into this, I've discovered that CallbackRunnable does not handle Throwables properly. It only handles NonFatals; as it is where all Future bodies and callbacks are run, it needs to handle all Throwables.

@NthPortal Behavior in the face of fatal exception is undefined. Which version are you referring to specifically? And what code are you saying should be handled differently?

@viktorklang This issue says the problem says that Futures are not completed for OOM and InterruptedException; in fact, they are not completed for any fatal Throwables.

Currently, the documentation for onFailure (soon onComplete) states that fatal exceptions/errors get wrapped in ExecutionExceptions. If that is not the case, and their behaviour is meant to be undefined (in this case never completed), then the documentation should be updated to reflect that.

I'm referring to 2.13, though the problem exists for 2.12 as well.

I was referring to run and executeWithValue (here for 2.12), which only catch NonFatals. Looking more closely, it's actually transform and transformWith which prevent the Future from completing (although it would be weird to handle fatals to complete Futures with them, but not report them in the ExecutionContext).

@NthPortal Where does the docs for the onFailure method say that fatals will get wrapped? (I read it a couple of times now but must have missed it or interpreted it differently)

@viktorklang It doesn't explicitly say all fatals get wrapped, but it mentions InterruptedException and ControlThrowable, which are both fatal

Edit: it also mentions Error, which is ambiguous. All the other fatals are Errors, so you'd probably expect them to be wrapped because of that, but it doesn't explicitly say all Errors

@NthPortal Yeah, the docs clearly needs to be more explicit about this. I'll have a look at that as a part of my Futures overhaul.

@viktorklang Another thing I noticed is that Promises cannot be completed with exceptions that happen in the process of execution, but before (or after, I guess) the body is executed. For example, if ExecutionContext.execute throws a RejectedExecutionException, the Promise/Future will never complete, though it should probably fail with said exception.

Personally, I think Futures should contain wrapped fatals (otherwise the Future never completes, and also the fatal gets ignored), but it's the community's decision how it should behave.

@NthPortal The first case (REE thrown) is fixed in the overhaul. The second case (auto-wrapping fatals) won't happen. If you want to have fatals propagate, you are free to add such wrapping explicitly (add the fatal as cause to a RuntimeException and throw that)

@viktorklang Okay, that seems pretty reasonable. I assume they will still get reported though

@NthPortal I just implemented and verified support for both interruptions and rejections here: viktorklang/scala-futures@2293c15#diff-823e12da79dd0256861a4903168fab59R344

I hope all mods will be ready in time for 2.13 but cannot guarantee it.

@viktorklang Cheers!

With the Future code in 2.13.0-M5 InterruptedExceptions should now be correctly handled.
Fatal errors should also be rethrown such that they either surface in the Executor(Service) and can be dealt with there. Keep in mind that all execution after a Fatal Error is undefined.

I'd more than welcome some feedback on the new Future handling here.