Off-stack error when running parallel action in same execution context

[felixSchl]

fiber <- fork do
  r <- try do
    -- delay $ 0.0 # Milliseconds -- uncomment this line and it will work
    -- OR remove the 'sequential $ parallel' part below and it will also work
    sequential $ parallel $ throwError $ error "boom"
  traceM r
liftAff $ killFiber (error "killit") fiber -- this causes the off-stack error
liftAff $ delay $ 1000.0 # Milliseconds -- required to see off-stack error

So, for some reason, when killing a fiber that has thrown an error inside a parallel action (and executed via sequential) that is immune to errors via try, the killing of the fiber causes an off-stack error to be produced. Breaking the execution context using delay will have the error go away. Similarly, not using sequential $ parallel at all will also make the error go away. Note that the error only surfaces when killing the fiber, and that you have to wait a little before the program terminates to witness the off-stack error.

[natefaubion]

I think the minimal reproduction is

test_regression_kill_rethrow ∷ Aff Unit
test_regression_kill_rethrow = assert "regression/par-apply-async-canceler" do
  ref ← newRef ""
  f1 ← forkAff $ makeAff \k -> k (Left (error "Boom.")) *> mempty
  killFiber (error "Nope.") f1

Which throws for me. I would expect this to exit cleanly, where f1 would have "Nope." as it's final exception value. The reason why this is interesting is the "synchronous" resolution of an async action with an exception, followed immediately by a kill.

When an async action is fulfilled, the continuation is put into the scheduler. https://github.com/slamdata/purescript-aff/blob/c02ae17e7da8c4df75b824c5b18911931bfe7eba/src/Effect/Aff.js#L324-L336

So this means that async actions are always resolved asynchronously. The sequential $ parallel $ throwError ... business is just a way of constructing an async action that immediately attempts to resolve with an error. The problem with the linked code is that the continuation unconditionally sets the state of the Fiber to STEP_RESULT with the resolved error. However in the test example, the fiber is killed in in between calling the async callback, and actually scheduling the fiber to continue. This is causing the fiber to resume when it should already be completed.

I think what's happening is:

1. Fork fiber A
2. Start evaluating A, which runs an async action that immediately resolves to an error E.
3. A's continuation is scheduled to run from error state E.
4. Parent fiber killed A with error F.
5. A's interrupt state is set to F, and some kill-related stuff is run.
6. A's original continuation is unconditionally resumed from error E, even though it has interrupt F.

So we probably need to guard the scheduled continuation against the interrupt state.

[natefaubion]

Changing the above to

            step   = runAsync(util.left, step._1, function (result) {
              return function () {
                if (runTick !== localRunTick) {
                  return;
                }
                runTick++;
                var currentInterrupt = interrupt;
                Scheduler.enqueue(function () {
                  if (interrupt !== currentInterrupt) {
                    return;
                  }
                  status = STEP_RESULT;
                  step   = result;
                  run(runTick);
                });
              };
            });

Fixes the issue for me, but there might be a better way to do the check.