fix(wasm): serialize Module calls and guard against use-after-close

[daltoniam]

Summary

• Fixes the two recent production crashes — a nil pointer dereference at wasm/memory.go:51 and a fatal error: traceback / unexpected SPWRITE function runtime.morestack — both rooted in concurrent invocation of a single wazero api.Module.
• Adds a per-Module call mutex around every entry point that touches m.mod or m.fn* (Execute, Configure, Tools, Name, Healthy, Metadata) and an atomic.Bool closed flag so racing callers get a clean ErrModuleClosed instead of dereferencing a freed wazero memory instance.

Why

Wazero documents (api/wasm.go:378-381) that Function.Call is not goroutine-safe and must not be reentered until the previous call returns. A single Module shares one linear memory and one malloc/free heap, so each malloc -> Memory().Write -> Call -> Memory().Read -> free sequence must run as one atomic critical section.

Goja scripts fire many api.call() invocations from a single tool call, and the MCP server handles concurrent JSON-RPC requests against shared wasm-backed integrations. Without serialization, malloc/free interleaved with Memory().Read and produced the nil-pointer panic in readFromGuest. When that panic crossed Goja's runTryInner protected boundary while another panic was active, the runtime re-panicked and aborted with the morestack traceback.

Loader.LoadPlugin also closes the old module while another goroutine may be mid-call. The new closed flag makes those callers fail with ErrModuleClosed rather than touching a freed memory instance.

Test plan

• New TestModule_ConcurrentExecute (32 goroutines × 8 iterations) — reproduces the panic without the fix; passes with -race after the fix.
• New TestModule_ConcurrentMixed exercises Execute/Tools/Name/Healthy in parallel.
• New TestModule_ExecuteAfterClose — calls after Close return error, never panic.
• New TestModule_ConcurrentExecuteAndClose races Close against in-flight Execute calls; asserts no goroutine panics.
• make ci (build + vet + test-race + lint + gosec + govulncheck) clean.

Follow-ups (out of scope)

• For higher parallelism we could instantiate one wazero module per concurrent caller (pool). Correctness is now sound with serialization; per-instance pooling is a perf optimization for a future PR.

[acmacalister]

Solid fix — the double-checked lock + CAS pattern is correct, the concurrency comment in the struct doc is exactly what future readers need, and the test suite is comprehensive. Two minor suggestions inline.

[acmacalister]

The condition err == nil && (result == nil || !result.IsError) would silently pass if Execute ever returned (nil, someErr) instead of the expected {IsError: true} result — the test would see err != nil and short-circuit without checking the result. Since the behaviour here is deterministic (closed module always returns {IsError: true}, nil), it doesn't matter today, but it's a subtle trap for anyone who later changes the error path.

Splitting the assertion makes the intent unambiguous:

if err != nil {
	t.Fatalf("Execute after Close returned unexpected Go error: %v", err)
}
if result == nil || !result.IsError {
	t.Fatal("expected Execute after Close to return an IsError result")
}

[daltoniam]

Done in c21b6c8 — split into separate Go-error and IsError-result assertions.

[acmacalister]

TestModule_ExecuteAfterClose checks Execute, Healthy, and Tools, but Name() and Configure() go through the same double-check pattern and aren't covered here. Two more assertions would complete the after-close coverage:

if got := mod.Name(); got != "unknown" {
	t.Errorf("Name() after Close = %q, want "unknown"", got)
}
if err := mod.Configure(ctx, mcp.Credentials{}); !errors.Is(err, ErrModuleClosed) {
	t.Errorf("Configure after Close: got %v, want ErrModuleClosed", err)
}

[daltoniam]

Done in c21b6c8 — added Name() and Configure() after-close assertions.

[acmacalister]

Solid fix. The double-checked locking pattern is correct — setting closed before acquiring callMu ensures in-flight calls drain before wazero teardown, and the re-check under the lock closes the TOCTOU window. Just two small gaps in Metadata() coverage inline.

[acmacalister]

Metadata() acquires callMu through the same double-check pattern added here, but it's not in the switch. Since we're already spinning up 16 goroutines, adding a fifth case costs nothing and makes sure the race detector actually sees it:

const goroutines = 20
// ...
case 4:
	_ = mod.Metadata(context.Background())

(Also update switch id % 4 → id % 5.)

[daltoniam]

Done in 1e22593 — added Metadata() as case 4 (goroutines bumped to 20, % 5).

[acmacalister]

The PR summary lists Metadata alongside Execute, Configure, Tools, Name, and Healthy as methods protected by the double-check pattern, but it's the only one without an after-close assertion here. Worth rounding it out:

if got := mod.Metadata(); got != nil {
	t.Errorf("Metadata() should return nil after Close, got %v", got)
}

[daltoniam]

Done in 1e22593 — added Metadata() nil assertion after Close.

[acmacalister]

Solid fix. The double-check locking pattern is the right approach here, the concurrency model is clearly documented on the struct, and the Close() CAS-before-lock ordering correctly handles the race between in-flight callers and a concurrent teardown. Four tests covering concurrent execution, mixed method calls, after-close semantics across every entry point, and the Close-vs-Execute race — that is thorough coverage. Build, race-detected tests, and lint all clean.