Skip to content
This repository was archived by the owner on Aug 12, 2026. It is now read-only.

Commit 3ccf845

Browse files
committed
Instance teardown completes even when a handle refuses to close
An adversarial review of 0017 found the bug by running it: a guest that traps leaves the instance unable to be entered, so every [resource-drop] then fails, and `Instance/close` did `(run! .close @registry)` before store_delete. The first failing drop escaped, so store_delete and arena.close never ran, and the retry was a silent no-op because `closed` was already set. That is the same shape as the bug the close ordering was written to fix two commits ago -- I moved the in-call check and left the handle loop inside the same guarded block, so the failure survived one level down. Teardown now always completes and the first failure is rethrown after it. dev/resources/res.{wit,wat} gains a trapping export, because the only way to reach this is a poisoned instance. The test is honest about its limits and says so in place: fault injection -- reverting to the plain `run!` -- leaves it green, because both versions throw and both leave `closed` set. What differs is whether store_delete and arena.close ran, and that is not observable from Clojure without adding API for a test. What it pins is the contract: close reports the drop rather than swallowing it, and the instance ends closed rather than stuck half-torn-down. The fix itself is justified by reading and by the review's C-level evidence. 180 assertions. Claude-Session: https://claude.ai/code/session_01XF5Hfq4Ca2N2XYEzWQQuHt
1 parent 52c587e commit 3ccf845

5 files changed

Lines changed: 175 additions & 4 deletions

File tree

‎dev/resources/res.wat‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
(local.set $v (i32.load (call $rep (local.get $h))))
4040
(call $drop (local.get $h))
4141
(local.get $v))
42+
(func (export "local:res/bag@0.1.0#boom") (result i32) (unreachable))
4243
(func (export "local:res/bag@0.1.0#make-two") (param $start i32) (result i32)
4344
(local $a i32) (local $b i32) (local $l i32)
4445
(local.set $a (call $alloc (i32.const 4)))

‎dev/resources/res.wit‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ interface bag {
1111
}
1212
/// Takes the resource: ownership transfers to the guest.
1313
consume: func(c: counter) -> u32;
14+
/// Traps. A trapped instance cannot be entered again, so every later
15+
/// [resource-drop] fails — which is how a handle's `close` comes to throw
16+
/// during the instance's own teardown.
17+
boom: func() -> u32;
1418
/// Handles the caller never destructured — `0016` D.
1519
make-two: func(start: u32) -> list<counter>;
1620
}

‎doc/design/0017-host-imports.md‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
# 0017 — Host imports: letting a component call Clojure
2+
3+
**Status:** proposed · 2026-07-30 · the mechanism is verified (`bb
4+
spike-import`); this is the API over it, and no code exists
5+
6+
## The question
7+
8+
`cljwit.host` can call a component. It cannot yet be called *by* one, which
9+
means it refuses every component that imports anything — including all of WASI.
10+
`0014` named this its most-likely-to-fire falsifier.
11+
12+
What is settled, measured 2026-07-30:
13+
14+
- **An FFM upcall stub reaches Clojure from inside a component call.** `bb
15+
spike-import`: `run(20) = 41`, the host saw `[20]`. Pure Clojure, bound to a
16+
`reify` — no bytecode generation and no C shim, the same answer `0011`
17+
reached for the outbound direction.
18+
- **A component's imports reflect exactly like its exports** — interface,
19+
parameter names, types, result — through
20+
`wasmtime_component_type_import_count`/`_nth`.
21+
- **A host callback cannot re-enter the instance that is executing**: `wasm
22+
trap: cannot enter component instance`. `0014` D and E survive.
23+
- **`wasmtime_component_linker_add_wasip2` supplies all of WASI 0.2 in one
24+
call**, so hand-writing `wasi:cli` is not the price of entry.
25+
- **`wasmtime_error_new(const char *)` exists**, so a Clojure exception has
26+
somewhere to go.
27+
28+
## The decision
29+
30+
### A. Imports are supplied at `instantiate`, keyed by the same names as exports
31+
32+
```clojure
33+
(host/instantiate art {:imports {"local:imp/host@0.1.0#twice" (fn [v] (* 2 v))}})
34+
```
35+
36+
The identity is the exact WIT name, `iface#func`, exactly as `0014` B decided
37+
for exports. One naming rule for both directions.
38+
39+
### B. The host reflects what the component needs and refuses a mismatch, both ways
40+
41+
A missing import throws at `instantiate`, **naming it**. wasmtime's own message
42+
is `function implementation is missing` with no name, which is a worse version
43+
of the `nil`-on-typo failure `0014` already rejected.
44+
45+
An import supplied but not needed throws too. A typo silently ignored is the
46+
same failure wearing the other hat.
47+
48+
### C. Marshalling is the same compiled lift and lower, run in the opposite direction
49+
50+
An import's parameters are *lifted* into Clojure and its result *lowered* back
51+
— the mirror of an export. `lift-fn` and `lower-fn` already compile from a
52+
reflected type tree; nothing new is needed but the direction.
53+
54+
### D. A Clojure exception becomes a `wasmtime_error_t *`, and never unwinds into native frames
55+
56+
The callback catches `Throwable`, converts it with `wasmtime_error_new`, and
57+
returns it. A JVM exception propagating through a native frame is undefined
58+
behaviour, and this project has already lost a JVM to a non-unwinding panic
59+
(`0012`'s surrogate `char`).
60+
61+
### E. WASI is a flag, not a map
62+
63+
```clojure
64+
(host/instantiate art {:wasi true})
65+
```
66+
67+
calls `add_wasip2`. Supplying a hundred functions by hand to run `wasi:cli` is
68+
not an API, and the C API already refuses to make you.
69+
70+
## Why
71+
72+
**A, because two naming rules would be one too many.** `0014` B paid for the
73+
exact-WIT-string identity with measurements about keywords; imports do not get
74+
to relitigate it.
75+
76+
**B, because instantiation is the only moment the host knows both sides.** It
77+
has the component's requirements from reflection and the user's map in hand. A
78+
mismatch discovered later is discovered by wasmtime, in a message with no name
79+
in it.
80+
81+
**D, because the failure mode is the worst one available.** Every other error
82+
in this library is an `ex-info`. An exception crossing into wasmtime's frames
83+
is a crash with no stack trace, and the cost of preventing it is a `try`.
84+
85+
## Alternatives rejected
86+
87+
- **Imports on the artifact rather than the instance.** The linker is
88+
engine-scoped, so this would be possible — and it would make every instance
89+
share one set of host functions, which is wrong the moment two requests want
90+
different state. `0014` C put per-request state on the instance.
91+
- **A separate `link` step** between `compile` and `instantiate`. Honest about
92+
where the linker lives, and a fourth lifetime for something that has no
93+
independent lifetime: nothing can use a linker except to instantiate.
94+
- **Letting a missing import fail lazily**, at the first call. Cheaper to
95+
implement and it moves the error away from the mistake.
96+
- **Requiring an explicit `wasi:cli` map even when `add_wasip2` exists.**
97+
Consistent, and unusable.
98+
- **Passing the raw `wasmtime_component_val_t` to the Clojure function.**
99+
Fastest, and it makes every import a chance to corrupt memory.
100+
101+
## What would falsify this
102+
103+
- **An import that is not a function.** A component may import a *type* or a
104+
*resource*; `wasmtime_component_item_t` has kinds for both. A: the map is
105+
keyed by function name and has nowhere to put them.
106+
- **The upcall stub's lifetime.** It is bound to an `Arena`, and it must
107+
outlive every call. If the instance's arena is the wrong scope — say the stub
108+
must outlive the *linker* rather than the instance — A is in the wrong place.
109+
- **`add_wasip2` needing a WASI context on the store.**
110+
`wasmtime_context_set_wasi` exists and takes a config; whether `add_wasip2`
111+
works without one, and what E should do about the config, is unverified.
112+
- **Whether a returned error surfaces to the outer caller as an error rather
113+
than a trap that poisons the instance.** D assumes it does. Unmeasured.
114+
- **The cost.** An upcall is not free and nothing here has been timed. If a
115+
guest calls a host import in a loop, the number that matters is per-upcall,
116+
and `0013`'s discipline applies: predict, then measure.
117+
118+
## Resources
119+
120+
- `dev/resources/imp.{wit,wat}` and `bb spike-import` — the mechanism.
121+
- `0014` — the naming rule, the lifetimes, and the concurrency check this
122+
inherits.
123+
- `0016` — the other direction of the same marshalling.

‎src/cljwit/host.clj‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -594,9 +594,20 @@
594594
(when (.compareAndSet closed false true)
595595
;; Before the store goes: a handle that outlives it holds a dangling
596596
;; context, which is the one real use-after-free in this row (0016 D).
597-
(run! (fn [^java.lang.AutoCloseable h] (.close h)) @registry)
598-
(invoke (:store-delete (.-api ^Engine (.-engine artifact))) store)
599-
(.close arena)))
597+
;;
598+
;; A handle's `drop` can throw — a guest that trapped leaves the
599+
;; instance unable to be entered, so every drop then fails. Letting
600+
;; that escape here would skip `store_delete` and `arena.close`
601+
;; entirely, and the retry would be a silent no-op because `closed` is
602+
;; already set: the same shape as the bug this ordering was written to
603+
;; fix, one level down. Teardown always completes; the first failure is
604+
;; rethrown after it.
605+
(let [errs (reduce (fn [acc ^java.lang.AutoCloseable h]
606+
(try (.close h) acc (catch Throwable t (conj acc t))))
607+
[] @registry)]
608+
(invoke (:store-delete (.-api ^Engine (.-engine artifact))) store)
609+
(.close arena)
610+
(when-let [t (first errs)] (throw t)))))
600611
nil)
601612
clojure.lang.ILookup
602613
(valAt [this k] (.valAt ^clojure.lang.ILookup this k ::none))

‎test/cljwit/host_test.clj‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,8 @@
267267
(is (= #{"local:res/bag@0.1.0#[constructor]counter"
268268
"local:res/bag@0.1.0#[method]counter.bump"
269269
"local:res/bag@0.1.0#consume"
270-
"local:res/bag@0.1.0#make-two"}
270+
"local:res/bag@0.1.0#make-two"
271+
"local:res/bag@0.1.0#boom"}
271272
(set (host/exports i)))))
272273

273274
(testing "own and borrow are distinct in the reflected type, not a runtime check"
@@ -338,3 +339,34 @@
338339
(is (= "#cljwit/resource[closed]" (str leaked))
339340
"the instance closed it on the way out"))))
340341
(finally (.delete ^File f))))))
342+
343+
(deftest teardown-completes-even-when-a-handle-refuses-to-close
344+
;; A trapped instance cannot be entered again, so every [resource-drop] then
345+
;; fails. If that escapes `Instance/close`, `store_delete` and `arena.close`
346+
;; never run and the retry is a silent no-op, because `closed` is already
347+
;; set — the same shape as the bug the close ordering was written to fix, one
348+
;; level down. Found by an adversarial review of `0017`, not by this suite.
349+
;;
350+
;; **This test does not distinguish the fix.** Fault injection: reverting to
351+
;; the plain `run!` leaves it green, because both versions throw and both
352+
;; leave `closed` set. What differs is whether `store_delete` and
353+
;; `arena.close` ran, and that is not observable from Clojure without adding
354+
;; API for a test. What it does pin is the contract — close reports the drop
355+
;; rather than swallowing it, and the instance ends closed rather than stuck
356+
;; half-torn-down.
357+
(if-not lib
358+
(println "CLJWIT_WASMTIME_LIB unset — skipping teardown test")
359+
(let [f (build-component! "res")]
360+
(try
361+
(with-open [e (host/engine)]
362+
(with-open [a (host/compile e (io/file f))]
363+
(let [i (host/instantiate a)]
364+
((i "local:res/bag@0.1.0#make-two") 1)
365+
(is (thrown? clojure.lang.ExceptionInfo ((i "local:res/bag@0.1.0#boom")))
366+
"the guest traps, which poisons the instance")
367+
(let [t (is (thrown? clojure.lang.ExceptionInfo (.close ^java.lang.AutoCloseable i)))]
368+
(is (re-find #"cannot enter component instance" (ex-message t))
369+
"close reports the drop that failed rather than swallowing it"))
370+
(is (nil? (.close ^java.lang.AutoCloseable i))
371+
"and the instance is closed afterwards, not stuck half-torn-down"))))
372+
(finally (.delete ^File f))))))

0 commit comments

Comments
 (0)