Skip to content

Serialize or synchronize concurrent synchronous calls on one Instance #4

Description

@JairusSW

Problem

Instance currently permits multiple goroutines to enter Call/CallExport on the same instance, and the implementation comments explicitly state that concurrent synchronous calls should remain race-free. Several call-lifecycle fields are nevertheless read or written without synchronization:

  • invoke reads target.poisoned;
  • trap/post-return paths write in.poisoned;
  • when syncTaskNeeded is true, every invokeEntered overwrites in.activeTask and in.syncBase, then restores prior values in a defer;
  • the async/exclusive/backpressure state is likewise instance-global and assumes a single active call chain.

Relevant code:

  • instance state and the concurrent-call claim: https://github.com/wago-org/component-model/blob/8a61139958e559a0ae61c3f49f6e9f33fddb6a95/internal/instance/instance.go
  • call entry / poison check:
    // particular call would otherwise even be well-formed.
    if target.poisoned {
    return nil, fmt.Errorf("component/instance: export %q: cannot enter component instance", exportName)
    }
    fd := be.fd
    if len(args) != len(fd.Params) {
    return nil, fmt.Errorf("component/instance: export %q takes %d parameter(s), got %d", exportName, len(fd.Params), len(args))
    }
    // be.coreFn/flattenErr etc. were resolved/computed once at bind time
    // (finalizeBoundExport) rather than here on every call -- see
    // boundExport's doc.
    if be.coreFn == nil {
    return nil, fmt.Errorf("component/instance: core module has no exported function %q (referenced by canon lift for export %q)", be.funcName, exportName)
    }
    if be.flattenErr != nil {
    return nil, fmt.Errorf("component/instance: export %q: flatten func type: %w", exportName, be.flattenErr)
    }
    return target.invokeEntered(ctx, be, exportName, args)
  • implicit sync-task installation:
    // code, so -- unlike a real trap escaping a CallWithStack -- it must not
    // poison; matches builtin-trap-poisons-instance's own two poisoning cases
    // (an `unreachable` and a busy-stream host-builtin trap), both of which
    // surface AS be.coreFn.CallWithStack failing, so this narrower rule still
    // covers everything that suite (or the spec) requires here.
    func (in *Instance) invokeEntered(ctx context.Context, be *boundExport, exportName string, args []abi.Value) ([]abi.Value, error) {
    if in.syncTaskNeeded {
    // The reference's canon_lift constructs a Task for EVERY call,
    // including a not-opts.async_ sync lift (definitions.py:2144-2202);
    // current_task() always resolves (:315-316). Install/restore around
    // the whole body (before lowerParams: a guest cabi_realloc invoked
    // during lowering may legally call context.get) so a nested
    // guest->guest sync call on this same Instance
    // (invokeEntered -> delegatingHostImport.fn -> invoke ->
    // invokeEntered) still resolves the innermost task, and the caller's
    // task is restored on return -- including through the two poisoning
    // error returns below, via defer.
    t := &task{inst: in, be: be, state: taskStarted, syncImplicit: true}
    prevActive, prevBase := in.activeTask, in.syncBase
    in.activeTask, in.syncBase = t, t

The core engine's execution lease does not make these fields safe: both goroutines can mutate component-level state before one of them reaches the serialized core entry, and poison is checked outside that lease.

Impact

Two ordinary host goroutines calling the same component can race. With current-task builtins enabled, call B can replace activeTask/syncBase while call A is lowering arguments or waiting to enter core code, causing builtins to resolve the wrong task and each defer to restore stale state. If either call traps, concurrent poison reads/writes are also a data race and another call may enter after the instance should have become poisoned.

This is directly reachable through consumers such as the WASI net/http.Handler, which naturally receives concurrent requests.

Suggested fix

Choose and document one model:

  1. Serialize public calls per Instance. Add an instance call gate covering the poison check, argument lowering (including guest realloc), core call, result lift/post-return, and implicit task installation. Preserve intentional same-call-chain re-entry with a scoped re-entry token rather than recursively taking the public gate.
  2. Support true concurrency. Move all call-local task state off Instance, make poison atomic/locked, and audit every scheduler/resource/lifecycle field for concurrent ownership. This is substantially larger.

For the current architecture, serialization is likely the safer initial contract.

Regression tests

Under go test -race:

  • overlap two synchronous exports on the same instance where syncTaskNeeded is true and each observes its own current-task/context state;
  • overlap a trapping export with a normal export and assert the normal call cannot enter after poisoning;
  • repeat with two CallExport calls through the same bound export;
  • verify intentional guest/host re-entry still works without deadlock.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions