Task.halt() can resolve before resource cleanup finishes

[dcolascione]

Summary

Task.halt() can resolve while async resource() cleanup is still blocked. That makes halt() report that shutdown is complete before the resource finalizer has actually finished.

This is observable with only public Effection APIs.

Reproduction

Tested with effection@4.0.2. I also checked effection-v4.1.0-alpha.7; the relevant resource(), Task.halt(), scope destruction, and Delimiter.exit() paths appear to have the same cancellation/cleanup shape.

import { resource, run, until } from "effection";

async function main(): Promise<void> {
  const events: string[] = [];
  const cleanupEntered = Promise.withResolvers<void>();
  const releaseCleanup = Promise.withResolvers<void>();
  const cleanupFinished = Promise.withResolvers<void>();

  function useResource() {
    return resource(function* resourceLifecycle(provide) {
      try {
        yield* provide("resource");
      } finally {
        events.push("cleanup:entered");
        cleanupEntered.resolve();

        yield* until(releaseCleanup.promise);

        events.push("cleanup:after-block");
        cleanupFinished.resolve();
      }
    });
  }

  const task = run(function* parentTask() {
    yield* useResource();
    events.push("main:return");
  });

  await cleanupEntered.promise;
  events.push("outside:calling-halt");

  await task.halt().then(
    () => events.push("halt:resolved"),
    (error: unknown) =>
      events.push(
        `halt:rejected:${
          error instanceof Error ? error.message : String(error)
        }`,
      ),
  );

  events.push("outside:releasing-cleanup");
  releaseCleanup.resolve();
  await cleanupFinished.promise;
  events.push("cleanup:finished-observed");

  await task.then(
    () => events.push("task:resolved"),
    (error: unknown) =>
      events.push(
        `task:rejected:${
          error instanceof Error ? error.message : String(error)
        }`,
      ),
  );

  console.log(events.join("\n"));
}

await main();

Actual output

main:return
cleanup:entered
outside:calling-halt
halt:resolved
outside:releasing-cleanup
cleanup:after-block
cleanup:finished-observed
task:rejected:halted

halt:resolved happens before outside:releasing-cleanup and before cleanup:after-block.

Expected behavior

task.halt() should not resolve until all shutdown associated with the task is complete, including async resource cleanup that is already in progress.

Expected ordering should put halt:resolved after cleanup:after-block / cleanup:finished-observed.

Why this matters

The Task.halt() docs say it returns a future that resolves only when all shutdown associated with the task is complete. Code that relies on await task.halt() as a join point can proceed while resource finalizers are still running.

That can produce leaked work, use-after-close races, and incorrect teardown ordering.

Notes

This looks related to the interaction between task scope destruction, encapsulate(... finally { yield* group.halt(); }), and Delimiter.exit() forcing coroutine completion via .return(). A second halt can cut across the in-progress cleanup join, making the parent shutdown path resolve even though a resource finalizer is still suspended and will continue later.

[cowboyd]

@dcolascione Thanks for taking the time to write this up. This is indeed a bug. We're having a look how we can simplify promise integration, but in the mean time, I do have a fix for this. I'm not super happy about it, in that I don't like accumulating state (versions 1 and 2 of Effection was a gigantic state machine, and it got unweildy very, very quickly), but until we can get to that, I think it's worth patching with a new bit.

[dcolascione]

Thanks. I think it's a bit symptomatic of not having a unified cancellation shielding story, TBH. I've had to patch Effection locally to get more predictable cleanups, and I've found shielding (combined with a lint rule to enforce its use) to be quite effective.