fix(wasm_runtime): issue WASM instance ids from one process-global allocator

[sanity]

Problem

wasm_runtime::delegate::test::test_large_secret_data and its siblings fail intermittently under the full parallel lib suite and never in isolation. Tracked as #4213 and #5023, with two distinct symptom shapes:

• test_large_secret_data / test_store_and_retrieve_secret: Expected SecretResult(Some(...)), got SecretResult(None)
• test_v2_delegate_update_existing_state: error_code: -1 / ContractNotFound

Both have the same cause.

MEM_ADDR, DELEGATE_ENV and CONTRACT_IO are process-global DashMaps keyed by WASM instance id, so the id namespace is process-global too. But WasmEngine::create_instance took a caller-supplied id: i64, and only RunningInstance::new drew one from the shared INSTANCE_ID counter. The engine unit tests in wasmtime_engine.rs picked their own ids:

• 0..10_001 in test_instance_limit_override_allows_many_instances
• 0..STORE_REFRESH_THRESHOLD (500) in the three store-refresh tests
• literals such as 999, 0, 1

Those tests' drop_instance calls do MEM_ADDR.remove(&id). Running in the same test binary as the delegate tests, they removed the entry belonging to a live delegate or contract instance in a concurrently-running test that had been issued the same id from the counter. Every host function on the victim instance then took its "no MEM_ADDR entry" branch and returned ERR_NOT_IN_PROCESS, which the stdlib's DelegateCtx collapses into "not found" (result < 0 → None).

That is why the flake needs the wasm_runtime:: or full-lib filter and has never reproduced under wasm_runtime::delegate alone (#5023 reports 24 consecutive clean runs there): the offending engine tests are filtered out.

Scope: the collision needs two tests to share a process, so it bites cargo test, which is what AGENTS.md tells contributors to run and what both issues reported. CI runs cargo nextest (ci.yml:546-556), which gives each test its own process, so CI was never affected.

Evidence

Captured with temporary instrumentation on the host-function early-return paths, which are otherwise tracing::warn!-only and invisible in a test run:

---- wasm_runtime::delegate::test::test_large_secret_data stdout ----
set_secret id=77 delegate=9XS1At... val_len=1048576 ok=true
refresh_mem_addr NO-MEM_ADDR-ENTRY id=89
get_secret EARLY no MEM_ADDR id=89
panicked: Expected SecretResult(Some(...)), got SecretResult(None)

---- wasm_runtime::delegate::test::test_v2_delegate_update_existing_state stdout ----
refresh_mem_addr NO-MEM_ADDR-ENTRY id=91
panicked: Expected ContractState, got ContractNotFound { ..., error_code: -1 }

get_secret_len for id 89 had already succeeded (no miss logged), so the entry existed and then vanished between two host calls on the same live instance. Both observed ids fall inside the 0..500 and 0..10_001 ranges the engine tests iterate.

Rate before the fix on this machine: 3 failures in 9 cargo test -p freenet --lib wasm_runtime:: runs, consistent with the ~1/8 in #5023.

Approach

Renumbering the offending tests would fix today's collision and leave the hazard for the next test that picks an id. Instead the collision is made unrepresentable:

• native_api.rs owns the single NEXT_INSTANCE_ID allocator, sited next to the global maps whose keyspace it defines, exposed as next_instance_id().
• WasmEngine::create_instance no longer takes an id. It allocates one and returns it in the InstanceHandle, so no caller can supply one.
• RunningInstance::new reads handle.id; the local INSTANCE_ID static is gone.
• Engine test call sites updated (mechanical: drop the id argument).

Production behaviour is unchanged: the same counter semantics, the same monotonic ids, the same single production caller (runtime.rs:73, still wrapped in classify_result per the #4864 invariant).

This closes the create_instance surface only. The WASM ABI is a separate id surface, called out in the NEXT_INSTANCE_ID docs so the invariant is not over-read.

Testing

Two new tests:

• instance_ids_are_globally_unique_across_engines: engine A holds a live instance with a recorded MEM_ADDR entry while engine B churns 64 instances the way the store-refresh and instance-limit tests do. Asserts B is never issued A's id and that A's entry survives. Mutation-tested: with create_instance reverted to a per-engine id (self.lifetime_instances as i64) it fails with left: 0, right: 0.
• create_instance_allocates_its_own_instance_id: bounded-region source pin, following the create_instance_recovers_store_on_guest_entry_failure convention already in this file. The type system stops a caller passing an id, but nothing stopped create_instance itself from reverting to a per-engine counter. Mutation-tested against exactly that change.

Verification: 14 consecutive full cargo test -p freenet --lib runs with zero wasm_runtime failures. Two of the 14 hit unrelated pre-existing flakes (cross_connection_median_returns_some_when_a_peer_has_inflation, already root-caused in #5039, and per_callsite_concurrent_writers_and_summary_no_deadlock, newly diagnosed in a comment there); the other 12 were fully green. A re-verification campaign on the final commit is in progress and the tally is posted in the discussion.

cargo fmt clean. cargo clippy -- -D warnings reports only pre-existing findings, none in the four files touched.

Closes #4213
Closes #5023

[AI-assisted - Claude]

[github-actions]

Rule Review: No issues found

Rules checked: git-workflow.md, code-style.md, testing.md, contracts.md
Files reviewed: 4 (engine.rs, engine/wasmtime_engine.rs, native_api.rs, runtime.rs)

This PR removes the caller-supplied instance id from WasmEngine::create_instance in favor of a single process-global AtomicI64 allocator (native_api::next_instance_id), fixing an id-collision bug across engines sharing one cargo test process. Checked against the applicable rules:

• testing.md (fix: PR regression-test requirement): satisfied — instance_ids_are_globally_unique_across_engines reproduces the exact collision scenario (two live engines, MEM_ADDR eviction) described in the bug.
• AGENTS.md source-scrape pin test pattern: create_instance_allocates_its_own_instance_id correctly bounds its scrape region (" fn create_instance(" → next "\n fn "), verified against the actual source that this uniquely matches the real trait-impl method (not a test fn) and stops cleanly at drop_instance.
• contracts.md (single wasmtime importer / one WasmEngine impl): unaffected — confirmed only WasmtimeEngine implements the trait.
• code-style.md: no .unwrap() introduced in production code; unused loop variables correctly renamed to _ where the removed id argument was their only use; import grouping unaffected.
• Doc comment's factual claim ("CI runs cargo nextest... CI was never affected") verified against .github/workflows/ci.yml — accurate.

No rule violations detected.

---

Rule review against .claude/rules/. WARNING findings block merge.

[sanity]

Review (Full tier: WASM runtime is a high-risk surface)

Two independent blind Claude reviewers read the checked-out code: a skeptical lens (adversarial bug hunt) and a code-first lens (read the code before the description, flag intent/implementation mismatches). No external model pass, per the current review policy.

The code-first reviewer found no discrepancies and independently confirmed the pin test in contract/executor.rs:2357 still holds after the signature change (create_calls == create_wrapped == 1). The skeptical reviewer raised six findings; all are addressed below.

Fixed in c986d9a

1. (High) The doc comment recorded the cause as more general than it is. The collision needs the two tests to share a process. CI runs cargo nextest, which gives each test its own process (.github/workflows/ci.yml:546-556), so CI was never affected. The flake bites cargo test, which is what AGENTS.md tells contributors to run and what both #4213 and #5023 actually reported (cargo test -p freenet --lib ... in each). Left unqualified, the rustdoc would have sent the next investigator down the wrong path if a delegate test ever flakes in CI. The doc now names the runner explicitly.

2. (Medium) "no caller can supply an id of its own" was too strong. Four host functions take an instance id as a guest-supplied WASM parameter and look it up in the same global maps: __frnt__logger__info, __frnt__rand__rand_bytes, __frnt__time__utc_now, __frnt__fill_buffer. That is a separate, pre-existing id surface this PR does not validate. The claim is now scoped to create_instance callers, with an explicit note that the WASM ABI is a distinct surface. The surface itself is out of scope here and has been reported through a non-public channel.

3. (Medium) The regression test's rustdoc oversold it. A caller-supplied id is now a compile error, not something a runtime test can catch. The rustdoc now claims only what the test pins: the allocator is process-global, not per-engine.

5. (Low) Enforcement moved from one site to every backend, with no pin. Added create_instance_allocates_its_own_instance_id, a bounded-region source scrape following the create_instance_recovers_store_on_guest_entry_failure convention already in this file. Mutation-tested: with create_instance reverted to self.lifetime_instances as i64 it fails with the intended message; with the fix it passes.

Not changed, with reasons

4. (Low) Gate drop_instance's MEM_ADDR.remove on self.instances.remove(...).is_some(). Declined. With ids now process-unique, drop_instance provably cannot reach another engine's entry, so the gate adds no safety. It does change behaviour on the replace_store() path, where self.instances is cleared while instances are still live: gating would skip the removal for those orphans and lean entirely on RunningInstance::drop. That is a real behavioural change on a high-risk surface in exchange for no demonstrated bug, which is the wrong trade in this PR.

6. (Low) replace_store() clears self.instances without removing those MEM_ADDR entries. Pre-existing, unreachable as a use-after-free (every host function refreshes the base pointer from its own Caller first), and out of scope for a flake fix. Noted so it is not lost.

Also verified by the reviewers

• All 20 call-site edits are mechanically correct, including the two that could have transposed arguments: (&module, 1, 0) → (&module, 0) and (&module, STORE_REFRESH_THRESHOLD as i64, 1024) → (&module, 1024).
• No remaining id source outside the allocator. create_instance is called only from runtime.rs:73 and this file's test module; nothing in crates/core/tests/ or simulation/.
• No test depended on the id it passed. SIMPLE_WASM makes no host calls, so 999, 999_999 and -1 were inert; the store-refresh tests assert on lifetime_instances, not ids.
• The -1 sentinel used by CURRENT_DELEGATE_INSTANCE is still unreachable: the allocator starts at 0 and only increments, and the one call site that used -1 is gone.
• Ordering is preserved: next_instance_id() is the first statement of create_instance, before __frnt_set_id tells the guest its id. A failed create burns an id exactly as the old INSTANCE_ID.fetch_add did.

Verification

14 consecutive full cargo test -p freenet --lib runs on f222c81b9: zero wasm_runtime failures. Two runs hit unrelated pre-existing flakes, both already tracked or being tracked: transport::rolling_rtt_stats::tests::cross_connection_median_returns_some_when_a_peer_has_inflation (#5039, root cause already identified there) and util::rate_limit_layer::tests::per_callsite_concurrent_writers_and_summary_no_deadlock.

For contrast, before the fix the narrower cargo test -p freenet --lib wasm_runtime:: loop reproduced the delegate flake 3 times in 9 runs on the same machine.

A re-verification campaign is running on c986d9a3d; the tally will be posted here.

[AI-assisted - Claude]

[sanity]

Re-verification on c986d9a3d

12 consecutive full cargo test -p freenet --lib runs on the final commit, --test-threads at the default (16 cores, machine load average 20-30 throughout from other concurrent work):

runs	wasm_runtime failures	other failures
12	0	1 (topology::small_world_rand::tests::chi_squared_test, a statistical test unrelated to this change)

Combined with the 14 runs on f222c81b9 reported above: 26 full-suite runs, zero occurrences of the flake. For contrast, before the fix the narrower cargo test -p freenet --lib wasm_runtime:: loop reproduced it 3 times in 9 runs on the same machine.

The other failures seen across the 26 runs were all pre-existing and unrelated: cross_connection_median_returns_some_when_a_peer_has_inflation (#5039, root cause already identified there), per_callsite_concurrent_writers_and_summary_no_deadlock (newly diagnosed in a comment on #5039), and the chi-squared test above.

[AI-assisted - Claude]