CABI: tighten subtask.cancel behavior, add tests

Author: lukewagner

design/mvp/CanonicalABI.md

@@ -584,7 +584,8 @@ time before or after the callee returns. If the callee returns and the
 `OnResolve` callback has *not* yet been called, the caller may invoke the
 returned `OnCancel` callback *at most once* to cooperatively request that the
 callee "hurry up" and call `OnResolve` (possibly, but not necessarily, passing
-`None` and/or skipping the call to `OnStart`).
+`None` and/or skipping the call to `OnStart`). The `OnCancel` may transitively
+execute arbitrary guest code but must not block.
 
 When `FuncInst` is implemented by wasm guest code (as opposed to the host), each
 call creates a `Task` object to track the state of the call and ensure that the
@@ -730,12 +731,20 @@ returned a value to its caller.
 ```
 
 The `Task.request_cancellation` method implements the `OnCancel` callback
-described above that allows a caller to indicate that they are no longer
+described above and allows a task's caller to indicate that they are no longer
 interested in the return value. If a task's implicit thread is waiting to start
 (in `Task.enter_implicit_thread`, defined above) due to backpressure, then it is
-immediately cancelled without running any guest code. Otherwise, the pending
-request is stored in the task state so that it can be picked up from the
-`async callback` event loop via `Task.deliver_pending_cancel`.
+immediately cancelled without running any guest code. Otherwise, if any of the
+threads in the callee's component instance are ready to run, one is resumed
+(chosen nondeterministically if there are multiple). Furthermore, the host may
+nondeterministically continue resuming ready threads in the same component
+instance until either the subtask resolves or the host declares that
+cancellation has blocked. Note that setting `Task.state` to `PENDING_CANCEL`
+makes any implicit `callback` thread contained by the task that is waiting in
+its event loop `ready` (as long as the component instance's `exclusive_thread`
+lock is not held by some other implicit thread). Thus, in the best case,
+`subtask.cancel` directly calls the subtask's `callback` function, passing
+`TASK_CANCELLED`.
 ```python
   def request_cancellation(self):
     if self.state == Task.State.INITIAL:
@@ -745,7 +754,18 @@ request is stored in the task state so that it can be picked up from the
     else:
       assert(self.state == Task.State.STARTED)
       self.state = Task.State.PENDING_CANCEL
+      while self.state != Task.State.RESOLVED:
+        candidates = { t for t in self.inst.threads if t.ready() }
+        if candidates:
+          random.choice(list(candidates)).resume()
+        if not candidates or DETERMINISTIC_PROFILE or random.randint(0,1):
+          break
+```
 
+If the pending cancellation request is not delivered to the subtask during
+`Task.request_cancellation`, it may still be delivered in the future to a
+`callback` via `Task.deliver_pending_cancel`:
+```python
   def has_pending_cancel(self):
     return self.state == Task.State.PENDING_CANCEL
 
@@ -4089,24 +4109,8 @@ validation specifies:
 * `$f` is given type `(func (param i32) (result i32))`
 * 🚝 - `async` is allowed (otherwise it must be absent)
 
-Calling `$f` sends a request to a nondeterministically-chosen thread of the
-subtask at the given index to cancel the subtask ASAP. This request is
-cooperative and the subtask may take arbitrarily long to receive and confirm
-the request. If the subtask doesn't immediately confirm the cancellation
-request, `subtask.cancel` returns `BLOCKED` and the caller must wait for a
-`SUBTASK` progress update using `waitable-set` methods as usual.
-
-When cancellation is confirmed the supertask will receive the final state of
-the subtask which is one of:
-* `RETURNED`, if the subtask successfully returned a value via `task.return`;
-* `CANCELLED_BEFORE_STARTED`, if the subtask was cancelled before receiving its
-  arguments (and thus no `own` handles were transferred); or
-* `CANCELLED_BEFORE_RETURNED`, if the subtask called `task.cancel` instead of
-  `task.return`.
-
-This state is either returned by `subtask.cancel`, if the subtask resolved
-without blocking, or, if `subtask.cancel` returns `BLOCKED`, then as part of
-the event payload of a future `SUBTASK` event.
+Calling `$f` invokes the following function which cooperatively requests
+cancellation of the given subtask:
 ```python
 BLOCKED = 0xffff_ffff
 
@@ -4122,11 +4126,8 @@ def canon_subtask_cancel(async_, i):
     subtask.cancellation_requested = True
     subtask.has_sync_waiter = True
     subtask.on_cancel()
-    if not subtask.resolved():
-      if not async_:
-        thread.wait_until(subtask.resolved)
-      else:
-        thread.yield_()
+    if not async_ and not subtask.resolved():
+      thread.wait_until(subtask.resolved)
     subtask.has_sync_waiter = False
     if not subtask.resolved():
       return [BLOCKED]
@@ -4148,26 +4149,28 @@ trapping on entry to `subtask.cancel`.
 
 Otherwise, if the subtask is *not* already resolved, the subtask's `OnCancel`
 callback is called to officially request cancellation. If the callee is another
-component, `OnCancel` is implemented by `Task.request_cancellation` which, as
-defined above, will immediately resolve a task that is blocked waiting to enter
-due to backpressure. Otherwise, the second `if not subtask.resolved()` handles
-the general case where core wasm code needs to run in the callee to receive and
-explicitly resolve the task (by calling `task.cancel` or `task.return`).
-
-If `subtask.cancel` is called synchronously, the call blocks until the subtask
-resolves. If `subtask.cancel` is called asynchronously, rather than *blocking*,
-`Thread.yield_` is called to allow the host to nondeterministically schedule
-whatever thread it heuristically chooses in the hopes that a subtask thread will
-get to execute and quickly resolve the subtask, allowing `subtask.cancel` to
-complete eagerly. Otherwise, `subtask.cancel` returns `BLOCKED` (`-1`) and the
-caller must wait for a `SUBTASK` event using a waitable set.
+component, `OnCancel` is implemented by `Task.request_cancellation` (defined
+above); which may execute arbitrary guest code in the callee before returning,
+but may not block.
+
+If the subtask is *not* eagerly resolved before `OnCancel` returns and
+`subtask.cancel` is called with the `async` immediate, then `subtask.cancel`
+immediately returns the sentinel `BLOCKED` (`-1`) value. Otherwise, if called
+without `async`, `subtask.cancel` blocks until the subtask *is* resolved. Once
+the subtask is resolved, the return value is the subtask's final `State` value
+which is one of:
+* `RETURNED`, if the subtask successfully returned a value via `task.return`;
+* `CANCELLED_BEFORE_STARTED`, if the subtask was cancelled before receiving its
+  arguments (and thus no `own` handles were transferred); or
+* `CANCELLED_BEFORE_RETURNED`, if the subtask called `task.cancel` instead of
+  `task.return`.
 
 Lastly, a subtle race condition guarded by the above code is that during the
-calls to `on_cancel`, `yield_` and `wait_until`, arbitrary code may run which
-can reenter the caller's component instance. By setting `has_sync_waiter` to
-true for the duration of these calls, `subtask.cancel` prevents other threads in
-the same component instance from "stealing" an event or otherwise invalidating
-the conditions guarded at the beginning of `subtask.cancel`.
+calls to `on_cancel` and `wait_until`, arbitrary code may run which can reenter
+the caller's component instance. By setting `has_sync_waiter` to true for the
+duration of these calls, `subtask.cancel` prevents other threads in the same
+component instance from "stealing" an event or otherwise invalidating the
+conditions guarded at the beginning of `subtask.cancel`.
 
 
 ### 🔀 `canon subtask.drop`

design/mvp/Concurrency.md

@@ -711,10 +711,11 @@ achieve the highest degree of (cooperative) concurrency.
 
 Since non-`async` functions are not allowed to block (including due to
 backpressure) and also don't pile up like `async` functions, non-`async`
-functions ignore backpressure (explicit and implicit) entirely. If a
-component exports a mix of `async` and non-`async` functions, code generation
-must therefore be prepared to handle non-`async` functions executing at
-any cooperative yield point, even in the middle of a `callback`.
+functions ignore backpressure (explicit and implicit) entirely. If a component
+exports a mix of `async` and non-`async` functions, code generation must
+therefore be prepared to handle non-`async` functions executing at any core wasm
+import call to a `canon` definition (with the exception of select non-reentrant
+built-ins like `resource.get`).
 
 Once a task is allowed to start according to these backpressure rules, its
 arguments are lowered into the callee's linear memory and the task is in
@@ -796,31 +797,29 @@ allowing the subtask to continue executing for an arbitrary amount of time
 to call [`task.cancel`] to confirm the cancellation or, for whatever reason,
 call `task.return` as-if there had been no cancellation. `task.cancel` enforces
 the same "all borrowed handles dropped" rule as `task.return`, so that once a
-subtask is resolved, the caller knows its lent handles have been returned. If
-the subtask was waiting to start due to backpressure, the subtask is immediately
-aborted without running the callee at all.
-
-When `subtask.cancel` is called, it first records a "pending cancellation
-request" on the callee's task. If the callee's task was waiting to start due to
-[backpressure](#backpressure), `subtask.cancel` returns "cancelled before
-started" immediately. Otherwise, `subtask.cancel` performs a nonblocking,
-cooperative `thread.yield` that allows the host to nondeterministically schedule
-any cooperative threads it wants in the hopes of delivering the cancellation
-request to the callee. Currently, cancellation requests can only be delivered
-to `async` functions using the stackless `callback` ABI. In this case, a pending
-cancellation request is delivered as a "task cancelled" event to the `callback`
-function, following the standard run-to-completion rules mentioned above. In
-the [future](#TODO), other cancellation delivery mechanisms will be added for
-the `async` functions using the stackful ABI.
-
-`subtask.cancel` can be called synchronously or asynchronously. If called
-synchronously, `subtask.cancel` will block until the subtask has resolved (i.e.,
-the subtask returns a value or calls `task.cancel`), returning whether the
-callee "returned" or "cancelled before returning". If called *asynchronously*
-and the callee has *still* not resolved after the host decides the cooperative
-yield has completed, `subtask.cancel` will return "blocked" instead and the
-caller must [wait](#waitables-and-waitable-sets) via waitable set until the
-subtask reaches a resolved state.
+subtask is resolved, the caller knows its lent handles have been returned.
+
+If the subtask was waiting to start due to [backpressure](#backpressure), the
+subtask is immediately aborted without running the callee at all, returning
+cancelled-before-started. Otherwise, `subtask.cancel` records the "pending
+cancellation request" in the subtask and attempts to resume execution in the
+subtask's component instance in the hopes that the subtask will quickly resolve
+itself. However, after at least one thread resumption that blocks, the host is
+free to declare that cancellation has blocked. In this case, asynchronous calls
+to `subtask.cancel` will immediately return a "blocked" code and synchronous
+calls to `subtask.cancel` will themselves block.
+
+The ready threads resumed by cancellation can be ready for all the normal
+reasons described above (yielding, I/O progress, etc). However, implicit threads
+using the `callback` ABI will *additionally* become ready due to the pending
+cancellation request itself (as long as the run-to-completion rules mentioned
+above are satisfied). If resumed, the `callback` will be passed a "task
+cancelled" event code to indicate the cancellation has been requested and that
+`task.cancel` may be called (instead of `task.return`). In the [future](#TODO),
+before the stackful ABI is released, other cancellation delivery mechanisms will
+be added so that threads via `waitable-set.wait` can also become ready and
+receive "task cancelled". Until then, `waitable-set.wait` will never return
+"task cancelled".
 
 The Component Model does not provide a mechanism to force prompt termination of
 threads as this can lead to leaks and corrupt state in a still-live component
@@ -870,8 +869,9 @@ defined by the Component Model:
   disabled, the order in which these pending tasks start, along with how
   they interleave with new tasks, is nondeterministic.
 * When `subtask.cancel` is called for a task that has started (i.e., passed the
-  backpressure gate), the host may nondeterministically resume as many or as few
-  threads as it wants before `subtask.cancel` returns.
+  backpressure gate), the choice of which ready thread to resume (if there are
+  multiple) and how many times to resume (if the first resumption blocks without
+  resolving the subtask) is nondeterministic.
 
 Despite the above, the following scenarios do behave deterministically:
 * If a component `a` asynchronously calls the export of another component `b`,

design/mvp/Explainer.md

@@ -1914,8 +1914,8 @@ resolved to (which is one of `returned`, `cancelled-before-started` or
 `cancelled-before-returned`).
 
 The `async` immediate is gated on 🚝. Without `async`, the `none` case is not
-possible and `subtask.cancel` synchronously waits until the callee is
-resolved.
+possible and `subtask.cancel` synchronously waits, if necessary, until the
+callee is resolved.
 
 For details, see [Cancellation] in the concurrency explainer and
 [`canon_subtask_cancel`] in the Canonical ABI explainer.

design/mvp/canonical-abi/definitions.py

@@ -467,6 +467,12 @@ def request_cancellation(self):
     else:
       assert(self.state == Task.State.STARTED)
       self.state = Task.State.PENDING_CANCEL
+      while self.state != Task.State.RESOLVED:
+        candidates = { t for t in self.inst.threads if t.ready() }
+        if candidates:
+          random.choice(list(candidates)).resume()
+        if not candidates or DETERMINISTIC_PROFILE or random.randint(0,1):
+          break
 
   def has_pending_cancel(self):
     return self.state == Task.State.PENDING_CANCEL
@@ -2367,11 +2373,8 @@ def canon_subtask_cancel(async_, i):
     subtask.cancellation_requested = True
     subtask.has_sync_waiter = True
     subtask.on_cancel()
-    if not subtask.resolved():
-      if not async_:
-        thread.wait_until(subtask.resolved)
-      else:
-        thread.yield_()
+    if not async_ and not subtask.resolved():
+      thread.wait_until(subtask.resolved)
     subtask.has_sync_waiter = False
     if not subtask.resolved():
       return [BLOCKED]

design/mvp/canonical-abi/run_tests.py

@@ -2518,28 +2518,19 @@ def core_caller(args):
 
     caller_heap.memory[0] = 13
     [ret] = canon_subtask_cancel(True, subi2)
-    if ret == definitions.BLOCKED:
-      canon_waitable_join(subi2, seti)
-      [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
-      assert(ret == EventCode.SUBTASK)
-      assert(caller_heap.memory[retp+0] == subi2)
-      assert(caller_heap.memory[retp+4] == Subtask.State.CANCELLED_BEFORE_RETURNED)
-    else:
-      assert(ret == Subtask.State.CANCELLED_BEFORE_RETURNED)
+    assert(ret == Subtask.State.CANCELLED_BEFORE_RETURNED)
     assert(caller_heap.memory[0] == 13)
     [] = canon_subtask_drop(subi2)
 
     caller_heap.memory[0] = 13
     [ret] = canon_subtask_cancel(True, subi3)
-    if ret == definitions.BLOCKED:
-      assert(caller_heap.memory[0] == 13)
-      [] = canon_waitable_join(subi3, seti)
-      [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
-      assert(ret == EventCode.SUBTASK)
-      assert(caller_heap.memory[retp+0] == subi3)
-      assert(caller_heap.memory[retp+4] == Subtask.State.RETURNED)
-    else:
-      assert(ret == Subtask.State.RETURNED)
+    assert(ret == definitions.BLOCKED)
+    assert(caller_heap.memory[0] == 13)
+    [] = canon_waitable_join(subi3, seti)
+    [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
+    assert(ret == EventCode.SUBTASK)
+    assert(caller_heap.memory[retp+0] == subi3)
+    assert(caller_heap.memory[retp+4] == Subtask.State.RETURNED)
     assert(caller_heap.memory[0] == 43)
     [] = canon_subtask_drop(subi3)
 
@@ -2551,15 +2542,13 @@ def core_caller(args):
 
     caller_heap.memory[0] = 13
     [ret] = canon_subtask_cancel(True, subi4)
+    assert(ret == definitions.BLOCKED)
     assert(caller_heap.memory[0] == 13)
-    if ret == definitions.BLOCKED:
-      [] = canon_waitable_join(subi4, seti)
-      [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
-      assert(ret == EventCode.SUBTASK)
-      assert(caller_heap.memory[retp+0] == subi4)
-      assert(caller_heap.memory[retp+4] == Subtask.State.CANCELLED_BEFORE_RETURNED)
-    else:
-      assert(ret == Subtask.State.CANCELLED_BEFORE_RETURNED)
+    [] = canon_waitable_join(subi4, seti)
+    [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
+    assert(ret == EventCode.SUBTASK)
+    assert(caller_heap.memory[retp+0] == subi4)
+    assert(caller_heap.memory[retp+4] == Subtask.State.CANCELLED_BEFORE_RETURNED)
     [] = canon_subtask_drop(subi4)
 
     caller_heap.memory[0] = 13
@@ -2587,14 +2576,7 @@ def core_caller(args):
     state,subi = unpack_result(ret)
     assert(state == Subtask.State.STARTED)
     [ret] = canon_subtask_cancel(True, subi)
-    if ret == definitions.BLOCKED:
-      canon_waitable_join(subi, seti)
-      [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
-      assert(ret == EventCode.SUBTASK)
-      assert(caller_heap.memory[retp+0] == subi)
-      assert(caller_heap.memory[retp+4] == Subtask.State.CANCELLED_BEFORE_RETURNED)
-    else:
-      assert(ret == Subtask.State.CANCELLED_BEFORE_RETURNED)
+    assert(ret == Subtask.State.CANCELLED_BEFORE_RETURNED)
     assert(caller_heap.memory[0] == 13)
     [] = canon_subtask_drop(subi)
 
@@ -2613,15 +2595,13 @@ def core_caller(args):
       state,subi_hog = unpack_result(ret)
       assert(state == Subtask.State.STARTED)
       [ret] = canon_subtask_cancel(True, subi)
-      if ret == definitions.BLOCKED:
-        assert(caller_heap.memory[0] == 13)
-        [] = canon_waitable_join(subi, seti)
-        [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
-        assert(ret == EventCode.SUBTASK)
-        assert(caller_heap.memory[retp+0] == subi)
-        assert(caller_heap.memory[retp+4] == cancelled_state)
-      else:
-        assert(ret == cancelled_state)
+      assert(ret == definitions.BLOCKED)
+      assert(caller_heap.memory[0] == 13)
+      [] = canon_waitable_join(subi, seti)
+      [ret] = canon_waitable_set_wait(caller_mem, seti, retp)
+      assert(ret == EventCode.SUBTASK)
+      assert(caller_heap.memory[retp+0] == subi)
+      assert(caller_heap.memory[retp+4] == cancelled_state)
       assert(caller_heap.memory[0] == expected)
       assert(caller_heap.memory[4] == 45)
       [] = canon_subtask_drop(subi)