CABI: redefine reentrance rules in terms of component instance flag

Author: lukewagner

design/mvp/CanonicalABI.md

@@ -123,6 +123,7 @@ class ComponentInstance:
   parent: Optional[ComponentInstance]
   handles: Table[ResourceHandle | Waitable | WaitableSet | ErrorContext]
   threads: Table[Thread]
+  may_enter: bool
   may_leave: bool
   backpressure: int
   num_waiting_to_enter: int
@@ -134,6 +135,7 @@ class ComponentInstance:
     self.parent = parent
     self.handles = Table()
     self.threads = Table()
+    self.may_enter = True
     self.may_leave = True
     self.backpressure = 0
     self.num_waiting_to_enter = 0
@@ -155,26 +157,37 @@ by the host, `None`, in the `parent` field. Thus, the set of component instances
 in a store forms a forest rooted by the component instances that were
 instantiated directly by the host.
 
-Based on this, the "reflexive ancestors" of a component instance (i.e., itself
-and all parent component instances up to the root component instance) can be
-enumerated and tested via these two helper functions:
+TODO
 ```python
-  def reflexive_ancestors(self) -> set[ComponentInstance]:
+  def enter_from(self, caller: Optional[ComponentInstance]):
+    for inst in self.entering(caller):
+      trap_if(not inst.may_enter)
+      inst.may_enter = False
+
+  def leave_to(self, caller: Optional[ComponentInstance]):
+    for inst in self.entering(caller):
+      assert(not inst.may_enter)
+      inst.may_enter = True
+
+  def entering(self, caller: Optional[ComponentInstance]):
+    if caller:
+      return self.self_and_ancestors() - caller.self_and_ancestors()
+    else:
+      return self.self_and_ancestors()
+
+  def self_and_ancestors(self) -> set[ComponentInstance]:
     s = set()
     inst = self
     while inst is not None:
       s.add(inst)
       inst = inst.parent
     return s
-
-  def is_reflexive_ancestor_of(self, other):
-    while other is not None:
-      if self is other:
-        return True
-      other = other.parent
-    return False
 ```
 
+Based on this, the "reflexive ancestors" of a component instance (i.e., itself
+and all parent component instances up to the root component instance) can be
+enumerated and tested via these two helper functions:
+...
 How the host instantiates and invokes root components is up to the host and not
 specified by the Component Model. Exports of previously-instantiated root
 components *may* be supplied as the imports of subsequently-instantiated root
@@ -206,76 +219,7 @@ cases are prevented via trap for several reasons:
   to link recursive calls and this requires opting in via some
   [TBD](Concurrency.md#TODO) function effect type or canonical ABI option.
 
-To detect and prevent recursive calls, the runtime tracks the dynamic call
-stack via a linked list of `Supertask` nodes. The `inst` field of `Supertask`
-either points to the `ComponentInstance` of the calling component or, if
-`None`, indicates that the caller is the host.
-```python
-class Supertask:
-  inst: Optional[ComponentInstance]
-  supertask: Optional[Supertask]
-```
-
-The `call_might_be_recursive` predicate is used by `Store.lift` to
-conservatively detect recursive reentrance and subsequently trap.
-```python
-def call_might_be_recursive(caller: Supertask, callee_inst: ComponentInstance):
-  if caller.inst is None:
-    while caller is not None:
-      if caller.inst and caller.inst.reflexive_ancestors() & callee_inst.reflexive_ancestors():
-        return True
-      caller = caller.supertask
-    return False
-  else:
-    return (caller.inst.is_reflexive_ancestor_of(callee_inst) or
-            callee_inst.is_reflexive_ancestor_of(caller.inst))
-```
-The first case (where `caller.inst` is `None`) covers host-to-component calls.
-By testing whether any of the callers' reflexive anecestor sets intersect the
-callee's ancestor set, the following case is considered recursive:
-```
-     +-------+
-     |   A   |<-.
-     | +---+ |  |
-host-->| B |-->host
-     | +---+ |
-     +-------+
-```
-Here, when attempting to recursively call back into `A`, `caller` points to the
-following stack:
-```
-|inst=None| --supertask--> |inst=B| --supertask--> |inst=None| --supertask--> None
-```
-while `A` does not appear as the `inst` of any `Supertask` on this stack,
-`B.reflexive_ancestors()` is `{ B, A }`, so the loop correctly determines that
-`A` is being reentered. This ensures that child components are kept an
-encapsulated detail of the parent.
-
-The second case (where `caller.inst` is not `None`) covers component-to-
-component calls by conservatively rejecting any call from a component to its
-anecestor or descendant (thereby preventing any possible recursion via ancestor
-`funcref`). Thus, the following sibling-to-sibling component call is allowed:
-```
-     +----------------+
-     |      P         |
-     | +----+  +----+ |
-host-->| C1 |->| C2 | |
-     | +----+  +----+ |
-     +----------------+
-```
-while the following child-to-parent and parent-to-child calls are disallowed:
-```
-     +----------+        +----------+
-     | +---+    |        |    +---+ |
-host-->| C |->P |  host->| P->| C | |
-     | +---+    |        |    +---+ |
-     +----------+        +----------+
-```
-This conservative approximation allows `call_might_be_recursive` to be computed
-ahead-of-time when compiling a fused component-to-component adapter (where both
-caller and callee intances and their relationship are statically known). In the
-future this check will be relaxed and more sophisticated optimizations can be
-used to statically eliminate the check in common cases.
+TODO
 
 The other fields of `ComponentInstance` are described below as they are used.
 
@@ -720,16 +664,14 @@ spec-level function type, where the host can be the caller, the callee or even
 OnStart = Callable[[], list[any]]
 OnResolve = Callable[[Optional[list[any]]], None]
 OnCancel = Callable[[], None]
-FuncInst = Callable[[Supertask, OnStart, OnResolve], OnCancel]
+FuncInst = Callable[[OnStart, OnResolve, Optional[ComponentInstance]], OnCancel]
 ```
 The three parameters of `FuncInst` are:
-* an optional caller `Supertask` which is used to maintain the
-  [async callstack][Structured Concurrency] and enforce the
-  non-reentrance [component invariant];
 * an `OnStart` callback that is called by the callee when it is ready to
   receive its arguments after waiting for any [backpressure] to subside;
 * an `OnResolve` callback that is called by the callee when it is ready to
   return its value or, if cancellation has been requested, `None`.
+* the caller's `ComponentInstance`, if the caller is not the host
 
 Critically, if the callee [blocks] at the wasm level, the spec-level `FuncInst`
 returns immediately to the caller while continuing to execute the callee in a
@@ -745,7 +687,7 @@ call creates a `Task` object to track the state of the call and ensure that the
 wasm guest code adheres to the above `FuncInst` calling convention (or else
 traps). `Task` is introduced in chunks, starting with fields and initialization:
 ```python
-class Task(Supertask):
+class Task:
   class State(Enum):
     INITIAL = 1
     STARTED = 2
@@ -756,19 +698,17 @@ class Task(Supertask):
   ft: FuncType
   opts: CanonicalOptions
   inst: ComponentInstance
-  supertask: Supertask
   on_start: OnStart
   on_resolve: OnResolve
   state: State
   num_borrows: int
   waiting_to_enter: Optional[Thread]
   threads: list[Thread]
 
-  def __init__(self, ft, opts, inst, supertask, on_start, on_resolve):
+  def __init__(self, ft, opts, inst, on_start, on_resolve):
     self.ft = ft
     self.opts = opts
     self.inst = inst
-    self.supertask = supertask
     self.on_start = on_start
     self.on_resolve = on_resolve
     self.state = Task.State.INITIAL
@@ -1013,44 +953,56 @@ CoreFuncInst = Callable[[list[CoreValType]], list[CoreValType]]
 
 class Store:
   waiting: list[Thread]
+  nesting_depth: int
 
   def __init__(self):
     self.waiting = []
+    self.nesting_depth = 0
 
-  def invoke(self, f: FuncInst, caller: Optional[Supertask], on_start, on_resolve) -> OnCancel:
-    host_caller = Supertask()
-    host_caller.inst = None
-    host_caller.supertask = caller
-    return f(host_caller, on_start, on_resolve)
+  def invoke(self, f: FuncInst, on_start: OnStart, on_resolve: OnResolve) -> OnCancel:
+    self.nesting_depth += 1
+    on_cancel = f(on_start, on_resolve, caller = None)
+    self.nesting_depth -= 1
+    return on_cancel
 
   def lift(self, f: CoreFuncInst, ft: FuncType, opts: CanonicalOptions, inst: ComponentInstance) -> FuncInst:
-    def func_inst(caller: Supertask, on_start: OnStart, on_resolve: OnResolve) -> OnCancel:
-      trap_if(call_might_be_recursive(caller, inst))
-      return canon_lift(f, ft, opts, inst, caller, on_start, on_resolve)
+    def func_inst(on_start: OnStart, on_resolve: OnResolve, caller: Optional[ComponentInstance]) -> OnCancel:
+      inst.enter_from(caller)
+      on_cancel = canon_lift(f, ft, opts, inst, on_start, on_resolve)
+      inst.leave_to(caller)
+      return on_cancel
     return func_inst
 
   def lower(self, f: FuncInst, ft: FuncType, opts: CanonicalOptions, inst: ComponentInstance) -> CoreFuncInst:
     def core_func_inst(args: list[CoreValType]) -> list[CoreValType]:
-      assert(current_instance() is inst)
+      assert(current_instance() is inst and self.nesting_depth > 0)
       return canon_lower(f, ft, opts, args)
     return core_func_inst
 
   def tick(self):
-    random.shuffle(self.waiting)
-    for thread in self.waiting:
-      if thread.ready():
-        thread.resume()
-        return
-```
+    assert(self.nesting_depth == 0)
+    candidates = { t for t in self.waiting if t.ready() }
+    if candidates:
+      thread = random.choice(list(candidates))
+      self.nesting_depth += 1
+      thread.task.inst.enter_from(None)
+      thread.resume()
+      thread.task.inst.leave_to(None)
+      self.nesting_depth -= 1
+```
+TODO: `nesting_depth`
+
 The `FuncInst` passed to `Store.invoke` is described above and can represent
 either a guest function (produced by `Store.lift`) or (in the special case of a
 component re-export of a host import) a host function. `Store.invoke` describes
 how a `FuncInst` is invoked by the host, but `FuncInst`s can also be invoked by
 guest code that calls the `CoreFuncInst` produced by `Store.lower`.
 
+TODO: update
 When a `FuncInst` is produced by lifting core wasm guest code, it is guarded
 by a call to `call_might_be_recursive`, which is described above.
 
+TODO: `nesting_depth == 0`
 The `Store.tick` method does not have an analogue in Core WebAssembly and
 enables [native concurrency support](Concurrency.md) in the Component Model. The
 expectation is that the host will interleave calls to `invoke` with calls to
@@ -3503,7 +3455,7 @@ executes in a new *implicit thread* defined here by `thread_func`. The first
 thing this implicit thread does is to wait for any backpressure, as defined by
 `Task.enter_implicit_thread` above:
 ```python
-def canon_lift(callee, ft, opts, inst, caller, on_start, on_resolve) -> OnCancel:
+def canon_lift(callee, ft, opts, inst, on_start, on_resolve) -> OnCancel:
   def thread_func():
     if not task.enter_implicit_thread():
       return
@@ -3635,7 +3587,7 @@ required by the `FuncInst` calling contract. Lastly, `canon_lift` returns
 `Task.request_cancellation`, bound to the call's new task, as the required
 `OnCancel` value.
 ```python
-  task = Task(ft, opts, inst, caller, on_start, on_resolve)
+  task = Task(ft, opts, inst, on_start, on_resolve)
   thread = Thread(task, thread_func)
   thread.resume()
   return task.request_cancellation
@@ -3775,7 +3727,7 @@ above).
       nonlocal flat_results
       flat_results = lower_flat_values(cx, max_flat_results, result, ft.result_type(), flat_args)
 
-  subtask.on_cancel = callee(thread.task, on_start, on_resolve)
+  subtask.on_cancel = callee(on_start, on_resolve, caller = thread.task.inst)
   assert(ft.async_ or subtask.state == Subtask.State.RETURNED)
 ```
 The `Subtask.state` field is updated by the callbacks to keep track of the
@@ -3899,17 +3851,13 @@ def canon_resource_drop(rt, i):
   trap_if(h.num_lends != 0)
   if h.own:
     assert(h.borrow_scope is None)
-    if inst is rt.impl:
-      if rt.dtor:
-        rt.dtor(h.rep)
-    else:
-      caller_opts = CanonicalOptions(async_ = False)
-      callee_opts = CanonicalOptions(async_ = rt.dtor_async, callback = rt.dtor_callback)
-      ft = FuncType([U32Type()],[], async_ = False)
-      dtor = rt.dtor or (lambda rep: [])
-      callee = inst.store.lift(dtor, ft, callee_opts, rt.impl)
-      caller = inst.store.lower(callee, ft, caller_opts, inst)
-      caller([h.rep])
+    caller_opts = CanonicalOptions(async_ = False)
+    callee_opts = CanonicalOptions(async_ = rt.dtor_async, callback = rt.dtor_callback)
+    ft = FuncType([U32Type()], [], async_ = False)
+    dtor = rt.dtor or (lambda rep: [])
+    callee = inst.store.lift(dtor, ft, callee_opts, rt.impl)
+    caller = inst.store.lower(callee, ft, caller_opts, inst)
+    caller([h.rep])
   else:
     h.borrow_scope.num_borrows -= 1
   return []
@@ -3920,15 +3868,12 @@ the private `i32` representation as a parameter. Thus, destructors *may* block
 on I/O, but only after they `task.return`, ensuring that `resource.drop` never
 blocks.
 
-Since there are valid reasons to call `resource.drop` in the same component
-instance that defined the resource, which would otherwise trap at the
-reentrance guard of `Store.lift`, an exception is made when the resource type's
-implementation-instance is the same as the current instance (which is
-statically known for any given `canon resource.drop`).
-
-When a destructor isn't present, there is still a trap on recursive reentrance
-since this is the caller's responsibility and the presence or absence of a
-destructor is an encapsulated implementation detail of the resource type.
+In particular, the `ComponentIntance.enter_from` method called by `Store.lift`
+may triger a trap if the call to the destructor would reenter the destructor's
+instance. In the special case where the `current_instance` is the *same* as the
+destructor's instance, `ComponentInstance.enter_from` is defined to *not* trap
+and thus, as one might expect, component instances can always `resource.drop`
+the owned handles that they created (via `resource.new`).
 
 
 ### `canon resource.rep`