From 4bcc7f526337a55670faa6b2fe140755f5021126 Mon Sep 17 00:00:00 2001 From: Ondra Pelech Date: Tue, 13 Oct 2026 20:43:06 +0200 Subject: [PATCH 1/3] go: don't call `OnDrop` twice when dropping an exported resource For a resource this component exports, `resource.drop` on an own handle runs the destructor synchronously (the canonical ABI calls the dtor when the dropping instance implements the resource). The generated `resourceDtor` already clears the handle, unpins the object and calls `OnDrop`, but `Drop()` then unpinned again and called `OnDrop` a second time. Rely on the destructor instead. Co-Authored-By: Claude Opus 5.5 --- crates/go/src/lib.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/go/src/lib.rs b/crates/go/src/lib.rs index d82c618f4..fad0d76ac 100644 --- a/crates/go/src/lib.rs +++ b/crates/go/src/lib.rs @@ -2994,11 +2994,11 @@ func (self *{camel}) SetHandle(handle int32) {{ func (self *{camel}) Drop() {{ handle := self.handle - if self.handle != 0 {{ - self.handle = 0 + if handle != 0 {{ + // Dropping an own handle of a resource this component exports runs + // its destructor (`resourceDtor{camel}`), which clears the handle, + // unpins `self` and calls `OnDrop`. resourceDrop{camel}(handle) - self.pinner.Unpin() - self.OnDrop() }} }} From 4c0cd16c76064d796311d14c929ddb2890297b5b Mon Sep 17 00:00:00 2001 From: Ondra Pelech Date: Tue, 13 Oct 2026 20:43:06 +0200 Subject: [PATCH 2/3] go: record the incoming handle in `FromOwnHandle` When an own handle of an exported resource is passed back into the component that exports it (e.g. `consume: func(x: x)`), the callee gets a new handle in its own table. `FromOwnHandle` returned the object without recording that handle, so `self.handle` still held the one `TakeHandle` handed out earlier. `Drop()` then released a stale handle (possibly another object's) and the incoming handle was never dropped. Record it. The new `resource-own-roundtrip` runtime test passes two own handles back at once, so they cannot both reuse the slot handed out before; it fails without this change. Co-Authored-By: Claude Opus 5.5 --- crates/go/src/lib.rs | 7 +++- .../runtime/resource-own-roundtrip/runner.go | 22 +++++++++++ .../runtime/resource-own-roundtrip/runner.rs | 21 ++++++++++ tests/runtime/resource-own-roundtrip/test.go | 31 +++++++++++++++ tests/runtime/resource-own-roundtrip/test.rs | 38 +++++++++++++++++++ tests/runtime/resource-own-roundtrip/test.wit | 24 ++++++++++++ 6 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 tests/runtime/resource-own-roundtrip/runner.go create mode 100644 tests/runtime/resource-own-roundtrip/runner.rs create mode 100644 tests/runtime/resource-own-roundtrip/test.go create mode 100644 tests/runtime/resource-own-roundtrip/test.rs create mode 100644 tests/runtime/resource-own-roundtrip/test.wit diff --git a/crates/go/src/lib.rs b/crates/go/src/lib.rs index fad0d76ac..5dc046392 100644 --- a/crates/go/src/lib.rs +++ b/crates/go/src/lib.rs @@ -3003,7 +3003,12 @@ func (self *{camel}) Drop() {{ }} func {camel}FromOwnHandle(handle int32) *{camel} {{ - return (*{camel})(unsafe.Pointer(resourceRep{camel}(handle))) + self := (*{camel})(unsafe.Pointer(resourceRep{camel}(handle))) + // The handle we received is a new one in this component's table, so + // record it; `Drop` must release this handle, not the one previously + // handed out by `TakeHandle`. + self.handle = handle + return self }} func {camel}FromBorrowHandle(rep int32) *{camel} {{ diff --git a/tests/runtime/resource-own-roundtrip/runner.go b/tests/runtime/resource-own-roundtrip/runner.go new file mode 100644 index 000000000..c3f1ee11c --- /dev/null +++ b/tests/runtime/resource-own-roundtrip/runner.go @@ -0,0 +1,22 @@ +package export_wit_world + +import ( + "fmt" + test "wit_component/test_resource_own_roundtrip_to_test" +) + +func Run() { + assertEqual(test.ConsumeBoth(test.MakeThing(1), test.MakeThing(2)), uint32(3)) + assertEqual(test.NumDropped(), uint32(2)) + + a := test.MakeThing(3) + b := test.MakeThing(4) + assertEqual(test.ConsumeBoth(b, a), uint32(7)) + assertEqual(test.NumDropped(), uint32(4)) +} + +func assertEqual[T comparable](a T, b T) { + if a != b { + panic(fmt.Sprintf("%v not equal to %v", a, b)) + } +} diff --git a/tests/runtime/resource-own-roundtrip/runner.rs b/tests/runtime/resource-own-roundtrip/runner.rs new file mode 100644 index 000000000..a5e8e12e5 --- /dev/null +++ b/tests/runtime/resource-own-roundtrip/runner.rs @@ -0,0 +1,21 @@ +include!(env!("BINDINGS")); + +use crate::test::resource_own_roundtrip::to_test::{consume_both, num_dropped, Thing}; + +struct Component; + +export!(Component); + +impl Guest for Component { + fn run() { + let a = Thing::new(1); + let b = Thing::new(2); + assert_eq!(consume_both(a, b), 3); + assert_eq!(num_dropped(), 2); + + let a = Thing::new(3); + let b = Thing::new(4); + assert_eq!(consume_both(b, a), 7); + assert_eq!(num_dropped(), 4); + } +} diff --git a/tests/runtime/resource-own-roundtrip/test.go b/tests/runtime/resource-own-roundtrip/test.go new file mode 100644 index 000000000..dea067445 --- /dev/null +++ b/tests/runtime/resource-own-roundtrip/test.go @@ -0,0 +1,31 @@ +package export_test_resource_own_roundtrip_to_test + +import ( + "runtime" +) + +type Thing struct { + pinner runtime.Pinner + handle int32 + val uint32 +} + +var numDropped uint32 = 0 + +func (self *Thing) OnDrop() { + numDropped++ +} + +func MakeThing(v uint32) *Thing { + return &Thing{runtime.Pinner{}, 0, v} +} + +func ConsumeBoth(a *Thing, b *Thing) uint32 { + defer a.Drop() + defer b.Drop() + return a.val + b.val +} + +func NumDropped() uint32 { + return numDropped +} diff --git a/tests/runtime/resource-own-roundtrip/test.rs b/tests/runtime/resource-own-roundtrip/test.rs new file mode 100644 index 000000000..e4f636889 --- /dev/null +++ b/tests/runtime/resource-own-roundtrip/test.rs @@ -0,0 +1,38 @@ +include!(env!("BINDINGS")); + +use exports::test::resource_own_roundtrip::to_test::{Guest, GuestThing, Thing}; +use std::sync::atomic::{AtomicU32, Ordering}; + +static NUM_DROPPED: AtomicU32 = AtomicU32::new(0); + +pub struct Test {} + +export!(Test); + +pub struct MyThing { + val: u32, +} + +impl Drop for MyThing { + fn drop(&mut self) { + NUM_DROPPED.fetch_add(1, Ordering::SeqCst); + } +} + +impl Guest for Test { + type Thing = MyThing; + + fn consume_both(a: Thing, b: Thing) -> u32 { + a.get::().val + b.get::().val + } + + fn num_dropped() -> u32 { + NUM_DROPPED.load(Ordering::SeqCst) + } +} + +impl GuestThing for MyThing { + fn new(v: u32) -> Self { + Self { val: v } + } +} diff --git a/tests/runtime/resource-own-roundtrip/test.wit b/tests/runtime/resource-own-roundtrip/test.wit new file mode 100644 index 000000000..c4c93f2e7 --- /dev/null +++ b/tests/runtime/resource-own-roundtrip/test.wit @@ -0,0 +1,24 @@ +package test:resource-own-roundtrip; + +// Own handles of an exported resource that are handed out and then passed +// back in. Passing two at once means the incoming handles cannot both reuse +// the slots handed out earlier, so the callee must drop the handles it +// actually received. +interface to-test { + resource thing { + constructor(v: u32); + } + + consume-both: func(a: thing, b: thing) -> u32; + num-dropped: func() -> u32; +} + +world test { + export to-test; +} + +world runner { + import to-test; + + export run: func(); +} From 2749e3469e01ec0e7da65d0a60882056e05b7b12 Mon Sep 17 00:00:00 2001 From: Ondra Pelech Date: Tue, 13 Oct 2026 20:44:20 +0200 Subject: [PATCH 3/3] go: use Go's standard generated-code header Go tools recognize generated files by a line matching `^// Code generated .* DO NOT EDIT\.$` (https://go.dev/s/generatedcode). The previous `// Generated by `wit-bindgen` X. DO NOT EDIT!` header did not match, so linters, `gopls` and GitHub's diff view treated the bindings as hand-written. Stub files keep their header since they are meant to be edited. Co-Authored-By: Claude Opus 5.5 --- crates/go/src/lib.rs | 58 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/go/src/lib.rs b/crates/go/src/lib.rs index 5dc046392..f9c779e33 100644 --- a/crates/go/src/lib.rs +++ b/crates/go/src/lib.rs @@ -930,7 +930,7 @@ impl WorldGenerator for Go { .collect::>() .join("\n"); let header = &format!( - "// Generated by `wit-bindgen` {version}. DO NOT EDIT! + "// Code generated by wit-bindgen {version}. DO NOT EDIT. // // This code was generated from the following packages: {packages} @@ -3473,3 +3473,59 @@ struct FutureStreamDedup { pkg_name: String, mangled_name: String, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn generated_files_have_go_generated_code_header() { + let mut resolve = Resolve::default(); + let pkg = resolve + .push_str( + "test.wit", + r#" + package a:b; + + interface i { + resource r { + constructor(); + } + f: func(x: r); + } + + world w { + import i; + export i; + } + "#, + ) + .unwrap(); + let world = resolve.select_world(&[pkg], Some("w")).unwrap(); + let mut files = Files::default(); + Opts { + format: Format::False, + ..Opts::default() + } + .build() + .generate(&mut resolve, world, &mut files) + .unwrap(); + + // Go tools recognize generated code by a line matching + // `^// Code generated .* DO NOT EDIT\.$` (https://go.dev/s/generatedcode). + let mut checked = 0; + for (name, contents) in files.iter() { + if !name.ends_with(".go") { + continue; + } + let first = std::str::from_utf8(contents).unwrap().lines().next(); + let first = first.unwrap_or_default(); + assert!( + first.starts_with("// Code generated ") && first.ends_with(" DO NOT EDIT."), + "{name}: unexpected first line {first:?}" + ); + checked += 1; + } + assert!(checked > 0); + } +}