bugfix: Tighten up memory model.

wasmer/config.go

@@ -87,22 +87,18 @@ func IsEngineAvailable(engine EngineKind) bool {
 
 // Config holds the compiler and the Engine used by the Store.
 type Config struct {
-	_inner *C.wasm_config_t
+	CPtrBase[*C.wasm_config_t]
 }
 
 // NewConfig instantiates and returns a new Config.
 //
 //	config := NewConfig()
 func NewConfig() *Config {
-	config := C.wasm_config_new()
-
-	return &Config{
-		_inner: config,
-	}
+	return &Config{CPtrBase: mkPtr(C.wasm_config_new())}
 }
 
 func (self *Config) inner() *C.wasm_config_t {
-	return self._inner
+	return self.ptr()
 }
 
 // UseUniversalEngine sets the engine to Universal in the configuration.
@@ -190,7 +186,9 @@ func metering_delegate(op C.wasmer_parser_operator_t) C.uint64_t {
 //			I32Add: 	4,
 //		 }
 //	  config.PushMeteringMiddleware(7865444, opmap)
-func (self *Config) PushMeteringMiddleware(maxGasUsageAllowed uint64, opMap map[Opcode]uint32) *Config {
+func (self *Config) PushMeteringMiddleware(
+	maxGasUsageAllowed uint64, opMap map[Opcode]uint32,
+) *Config {
 	if opCodeMap == nil {
 		// REVIEW only allowing this to be set once
 		opCodeMap = opMap
@@ -278,7 +276,7 @@ func (self *Config) UseSinglepassCompiler() *Config {
 //	config := NewConfig()
 //	config.UseTarget(target)
 func (self *Config) UseTarget(target *Target) *Config {
-	C.wasm_config_set_target(self.inner(), target.inner())
+	C.wasm_config_set_target(self.inner(), target.release())
 
 	return self
 }

wasmer/config_test.go

@@ -29,13 +29,16 @@ func TestConfig(t *testing.T) {
 	engine := NewEngineWithConfig(config)
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
+	defer runtime.KeepAlive(module)
+
 	assert.NoError(t, err)
 
 	instance, err := NewInstance(module, NewImportObject())
 	assert.NoError(t, err)
 
-	sum, err := instance.Exports.GetFunction("sum")
+	sum, release, err := instance.GetFunctionSafe("sum")
 	assert.NoError(t, err)
+	defer release(instance)
 
 	result, err := sum(37, 5)
 	assert.NoError(t, err)
@@ -53,12 +56,14 @@ func TestConfigForMetering(t *testing.T) {
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
 	assert.NoError(t, err)
+	defer runtime.KeepAlive(module)
 
 	instance, err := NewInstance(module, NewImportObject())
 	assert.NoError(t, err)
 
-	sum, err := instance.Exports.GetFunction("sum")
+	sum, release, err := instance.GetFunctionSafe("sum")
 	assert.NoError(t, err)
+	defer release(instance)
 
 	result, err := sum(37, 5)
 	assert.NoError(t, err)
@@ -74,12 +79,14 @@ func TestConfigForMeteringFn(t *testing.T) {
 	store := NewStore(engine)
 	module, err := NewModule(store, testGetBytes("tests.wasm"))
 	assert.NoError(t, err)
+	defer runtime.KeepAlive(module)
 
 	instance, err := NewInstance(module, NewImportObject())
 	assert.NoError(t, err)
 
-	sum, err := instance.Exports.GetFunction("sum")
+	sum, release, err := instance.GetFunctionSafe("sum")
 	assert.NoError(t, err)
+	defer release(instance)
 
 	result, err := sum(37, 5)
 	assert.NoError(t, err)
@@ -144,12 +151,14 @@ func TestConfig_AllCombinations(t *testing.T) {
 				store := NewStore(engine)
 				module, err := NewModule(store, testGetBytes("tests.wasm"))
 				assert.NoError(t, err)
+				defer runtime.KeepAlive(module)
 
 				instance, err := NewInstance(module, NewImportObject())
 				assert.NoError(t, err)
 
-				sum, err := instance.Exports.GetFunction("sum")
+				sum, release, err := instance.GetFunctionSafe("sum")
 				assert.NoError(t, err)
+				defer release(instance)
 
 				result, err := sum(37, 5)
 				assert.NoError(t, err)

wasmer/cptr.go

@@ -0,0 +1,33 @@
+package wasmer
+
+// CPtrBase is a based struct for a C pointer.
+// It is intended to be embedded into any structure
+// that stores a C pointer.
+// While this based is parameterized on type any, using any
+// type other than *C.xxx pointer should be considered an undefined behavior.
+type CPtrBase[T comparable] struct {
+	_ptr        T
+	maybeStack  // stack of the creating goroutine (if enabled via memcheck)
+	maybeNil[T] // indicates if initial value of _ptr was nil (if enabled via memcheck)
+}
+
+// release returns the C pointer stored in this base and clears the finalizer.
+func (b *CPtrBase[T]) release() T {
+	var zero T
+	v := b.ptr()
+	b._ptr = zero
+	b.ClearFinalizer()
+	return v
+}
+
+func (b *CPtrBase[T]) SetFinalizer(free func(v T)) {
+	doSetFinalizer(b, free)
+}
+
+func (b *CPtrBase[T]) ClearFinalizer() {
+	doClearFinalizer(b)
+}
+
+func mkPtr[T comparable](ptr T) CPtrBase[T] {
+	return doMkPtr(ptr)
+}

wasmer/cptr_check.go

@@ -0,0 +1,138 @@
+//go:build memcheck
+
+package wasmer
+
+import (
+	"bytes"
+	"fmt"
+	"os"
+	"os/signal"
+	"runtime"
+	"sync"
+	"sync/atomic"
+	"syscall"
+	"unsafe"
+)
+
+func doMkPtr[T comparable](ptr T) CPtrBase[T] {
+	var zero T
+	b := CPtrBase[T]{_ptr: ptr}
+	b._stack = stack(3)
+	b.maybeNil._nil = ptr == zero
+	b.maybeNil._zero = zero
+	return b
+}
+
+type maybeNil[T comparable] struct {
+	_nil  bool
+	_zero T
+}
+
+// maybeStack stores the stack of the goroutine.
+type maybeStack struct {
+	_stack []byte
+}
+
+// ptr returns the C pointer stored in this base.
+func (b *CPtrBase[T]) ptr() T {
+	if !b.maybeNil._nil && b._ptr == b.maybeNil._zero {
+		panic(fmt.Errorf("attempt to use a released object; ptr was allocated by\n%s", b._stack))
+	}
+	runtime.GC()
+	return b._ptr
+}
+
+func doSetFinalizer[T comparable](b *CPtrBase[T], free func(v T)) {
+	finalizers.Store(uintptr(unsafe.Pointer(b)), stack(3))
+
+	runtime.SetFinalizer(b, func(b *CPtrBase[T]) {
+		ptr := b._ptr
+		b._ptr = b.maybeNil._zero
+		b.maybeNil._nil = false
+		free(ptr)
+		finalizers.Delete(uintptr(unsafe.Pointer(b)))
+	})
+}
+
+func doClearFinalizer[T comparable](b *CPtrBase[T]) {
+	runtime.SetFinalizer(b, nil)
+	finalizers.Delete(uintptr(unsafe.Pointer(b)))
+}
+
+var finalizers sync.Map // uintptr -> []byte (the stack of the caller which created the finalizer)
+
+func init() {
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, syscall.SIGABRT, syscall.SIGSEGV, syscall.SIGBUS, syscall.SIGINT)
+
+	go func() {
+		s := <-c
+		fmt.Fprintf(os.Stderr, "Signal %s received\n", s)
+
+		allStacks := make([]byte, 1<<18)
+		sl := runtime.Stack(allStacks, true)
+		if sl == len(allStacks) {
+			// Try again, once, with a bigger buffer
+			allStacks = make([]byte, 1<<20)
+			sl = runtime.Stack(allStacks, true)
+		}
+		allStacks = allStacks[:sl]
+
+		fmt.Fprintf(os.Stderr, "\n%s\n\n", allStacks)
+		fmt.Fprintf(os.Stderr, "Pending Finalizers:\n")
+
+		n := 0
+		finalizers.Range(func(k, v interface{}) bool {
+			n++
+			stack := v.([]byte)
+			fmt.Fprintf(os.Stderr, "\n--finilizer %d: %s\n\n", n, stack)
+			return true
+		})
+		os.Exit(1)
+	}()
+}
+
+// stackCache seems to be a bit of an overkill for debug build, but
+// some of the tests (e.g. TestSumLoop) allocate many pointers; and
+// the time and memory used for that adds up.
+const stackCacheMaxSize = 1 << 9
+
+var stackCache sync.Map // uintptr -> []byte
+var stackCacheSize atomic.Int32
+
+func stack(skip int) []byte {
+	var buf bytes.Buffer
+	const maxDepth = 32
+	pc := make([]uintptr, maxDepth)
+	// Skip the requested number of frames + the frames for this function
+	n := runtime.Callers(skip+1, pc)
+	if n == 0 {
+		return nil
+	}
+	pc = pc[:n]
+
+	if v, ok := stackCache.Load(pc[0]); ok {
+		return v.([]byte)
+	}
+
+	// Convert program counters to frames
+	frames := runtime.CallersFrames(pc)
+	for {
+		frame, more := frames.Next()
+		// Format the frame as "file:line function"
+		fmt.Fprintf(&buf, "%s:%d %s\n", frame.File, frame.Line, frame.Function)
+		if !more {
+			break
+		}
+	}
+
+	s := buf.Bytes()
+	stackCache.Store(pc[0], s)
+	if stackCacheSize.Add(1) > stackCacheMaxSize {
+		stackCache.Range(func(k, v interface{}) bool {
+			stackCache.Delete(k)
+			return stackCacheSize.Add(-1) > (stackCacheMaxSize >> 1)
+		})
+	}
+	return s
+}

wasmer/cptr_nocheck.go

@@ -0,0 +1,32 @@
+//go:build !memcheck
+
+package wasmer
+
+import "runtime"
+
+func doMkPtr[T comparable](ptr T) CPtrBase[T] {
+	return CPtrBase[T]{_ptr: ptr}
+}
+
+// maybeStack is a no-op implementation for storing
+// ptr creator stack.
+type maybeStack struct{}
+
+// maybeNil is a no-op implementation for indicating
+// if initial value of _ptr was nil.
+type maybeNil[T comparable] struct{}
+
+// ptr returns the C pointer stored in this base.
+func (b *CPtrBase[T]) ptr() T {
+	return b._ptr
+}
+
+func doSetFinalizer[T comparable](b *CPtrBase[T], free func(v T)) {
+	runtime.SetFinalizer(b, func(b *CPtrBase[T]) {
+		free(b.ptr())
+	})
+}
+
+func doClearFinalizer[T comparable](b *CPtrBase[T]) {
+	runtime.SetFinalizer(b, nil)
+}

wasmer/engine.go

@@ -2,23 +2,18 @@ package wasmer
 
 // #include <wasmer.h>
 import "C"
-import "runtime"
 
 // Engine is used by the Store to drive the compilation and the
 // execution of a WebAssembly module.
 type Engine struct {
-	_inner *C.wasm_engine_t
+	CPtrBase[*C.wasm_engine_t]
 }
 
 func newEngine(engine *C.wasm_engine_t) *Engine {
-	self := &Engine{
-		_inner: engine,
-	}
-
-	runtime.SetFinalizer(self, func(self *Engine) {
-		C.wasm_engine_delete(self.inner())
+	self := &Engine{CPtrBase: mkPtr(engine)}
+	self.SetFinalizer(func(v *C.wasm_engine_t) {
+		C.wasm_engine_delete(v)
 	})
-
 	return self
 }
 
@@ -58,7 +53,7 @@ func NewDylibEngine() *Engine {
 }
 
 func (self *Engine) inner() *C.wasm_engine_t {
-	return self._inner
+	return self.ptr()
 }
 
 // NewJITEngine is a deprecated function. Please use NewUniversalEngine instead.

wasmer/engine_test.go

@@ -9,14 +9,16 @@ import (
 
 func testEngine(t *testing.T, engine *Engine) {
 	store := NewStore(engine)
-	module, err := NewModule(store, testGetBytes("tests.wasm"))
+	module, modrelease, err := NewModuleSafe(store, testGetBytes("tests.wasm"))
 	assert.NoError(t, err)
+	defer modrelease(module)
 
 	instance, err := NewInstance(module, NewImportObject())
 	assert.NoError(t, err)
 
-	sum, err := instance.Exports.GetFunction("sum")
+	sum, release, err := instance.GetFunctionSafe("sum")
 	assert.NoError(t, err)
+	defer release(instance)
 
 	result, err := sum(37, 5)
 	assert.NoError(t, err)
@@ -44,7 +46,6 @@ func TestEngineWithTarget(t *testing.T) {
 	assert.NoError(t, err)
 
 	cpuFeatures := NewCpuFeatures()
-	assert.NoError(t, err)
 
 	target := NewTarget(triple, cpuFeatures)