🐛 fix: move all task finalization into the scope destructor (#1085)

We need to have 1:1 equivalance between a scope closing and a task
settling.

- when a task errors or completes, its scope must be destroyed
- when a task is halted, its scope must be destroyed.

By the same token, when scope is destroyed, its task must halt (if
there is such a task).

With #1081, we established the scope destroy -> task halt
relationship, and now we need to establish the task finalized -> scope
destroyed relatioship.

This moves all of the finalization logic of the task from the actual
coroutine running the task into the scope destructor. Then, the
coroutine itself _explicitly_ destroys the scope as its last official
act. To make this work, we hid need to make Delimiter.close()
re-entrant, so that if it is called from mulitple places it is
idempotent and has the same result.

Author: cowboyd

lib/delimiter.ts

@@ -36,14 +36,21 @@ export class Delimiter<T>
   *close(): Operation<void> {
     let done = this.future.operation;
     let interrupted = !this.computed;
+
     this.close = function* close() {
       let outcome = yield* done;
       if (interrupted && outcome.exists && !outcome.value.ok) {
         throw outcome.value.error;
       }
     };
-    this.interrupt();
-    yield* this.close();
+    if (!this.outcome) {
+      this.interrupt();
+      yield* this.close();
+    } else {
+      if (interrupted && this.outcome.exists && !this.outcome.value.ok) {
+        throw this.outcome.value.error;
+      }
+    }
   }
 
   private exit(outcome: Maybe<Result<T>>): void {

lib/task.ts

@@ -26,17 +26,6 @@ export function createTask<T>(options: TaskOptions<T>): NewTask<T> {
   let [scope, destroy] = createScopeInternal(owner);
   let future = createFuture<T>();
 
-  let top = new Delimiter<T>(() => encapsulate(operation));
-  scope.set(DelimiterContext, top as Delimiter<unknown>);
-
-  scope.ensure(function* () {
-    try {
-      yield* top.close();
-    } finally {
-      future.reject(new Error("halted"));
-    }
-  });
-
   let task = Object.defineProperties(future.future, {
     halt: {
       enumerable: false,
@@ -77,33 +66,43 @@ export function createTask<T>(options: TaskOptions<T>): NewTask<T> {
     },
   }) as Task<T>;
 
-  let boundary = owner.expect(ErrorContext);
-
-  scope.set(ErrorContext, top);
+  let top = new Delimiter<T>(() => encapsulate(operation));
+  scope.set(DelimiterContext, top as Delimiter<unknown>);
 
   let group = scope.expect(TaskGroupContext);
   group.add(task);
 
+  let boundary = owner.expect(ErrorContext);
+  scope.set(ErrorContext, top);
+
+  scope.ensure(function* () {
+    try {
+      yield* top.close();
+    } finally {
+      group.delete(task);
+      let { outcome } = top;
+      if (outcome!.exists) {
+        let result = outcome!.value;
+        if (result.ok) {
+          future.resolve(result.value);
+        } else {
+          let { error } = result;
+          future.reject(error);
+          boundary.raise(error);
+        }
+      } else {
+        future.reject(new Error("halted"));
+      }
+    }
+  });
+
   let routine = createCoroutine({
     scope,
     *operation() {
       try {
         yield* top;
       } finally {
-        group.delete(task);
-        let { outcome } = top;
-        if (outcome!.exists) {
-          let result = outcome!.value;
-          if (result.ok) {
-            future.resolve(result.value);
-          } else {
-            let { error } = result;
-            future.reject(error);
-            boundary.raise(error);
-          }
-        } else {
-          future.reject(new Error("halted"));
-        }
+        yield* destroy();
       }
     },
   });

test/run.test.ts

@@ -1,6 +1,6 @@
 // deno-lint-ignore-file no-unsafe-finally
-
-import { blowUp, createNumber, describe, expect, it } from "./suite.ts";
+import assert from "node:assert";
+import { Children } from "../lib/contexts.ts";
 import {
   action,
   createScope,
@@ -13,8 +13,7 @@ import {
   until,
   useScope,
 } from "../mod.ts";
-import { Children } from "../lib/contexts.ts";
-import assert from "node:assert";
+import { blowUp, createNumber, describe, expect, it } from "./suite.ts";
 
 describe("run()", () => {
   it("can run an operation", async () => {
@@ -335,9 +334,20 @@ describe("run()", () => {
       createScope(scope);
       yield* suspend();
     });
+
     await task.halt();
 
     assert(scope !== undefined);
     expect(scope.expect(Children).size).toEqual(0);
   });
+
+  it("destroys its scope when completing successfully", async () => {
+    let scope = await run(function* () {
+      let parent = yield* useScope();
+      createScope(parent);
+      return parent;
+    });
+
+    expect(scope.expect(Children).size).toEqual(0);
+  });
 });