Repository: tetratelabs/wazero
Pull request #375 (2022-03-15 11:59:40)

Propagates error return Close to public API
Before, we only handled errors internally and swallowed them when re-exporting to the public API.


Patch:
diff --git a/internal/wasm/module_context.go b/internal/wasm/module_context.go
index 3dc36c46..7fd48001 100644
--- a/internal/wasm/module_context.go
+++ b/internal/wasm/module_context.go
@@ -57,8 +57,9 @@ func (m *ModuleContext) WithContext(ctx context.Context) publicwasm.Module {
 	return m
 }
 
-func (m *ModuleContext) Close() {
-	m.store.CloseModule(m.module.Name)
+// Close implements io.Closer
+func (m *ModuleContext) Close() error {
+	return m.store.CloseModule(m.module.Name)
 }
 
 // Memory implements wasm.Module Memory
diff --git a/internal/wasm/store.go b/internal/wasm/store.go
index af859d06..6e44c323 100644
--- a/internal/wasm/store.go
+++ b/internal/wasm/store.go
@@ -338,12 +338,13 @@ func (s *Store) Instantiate(ctx context.Context, module *Module, name string) (*
 }
 
 // CloseModule deallocates resources if a module with the given name exists.
-func (s *Store) CloseModule(moduleName string) {
+func (s *Store) CloseModule(moduleName string) (err error) {
 	m := s.module(moduleName)
 	if m != nil {
-		m.Engine.Close()
+		err = m.Engine.Close()
 		s.deleteModule(moduleName)
 	}
+	return
 }
 
 // deleteModule makes the moduleName available for instantiation again.
diff --git a/wasm/wasm.go b/wasm/wasm.go
index 5a8e196b..917b1fd5 100644
--- a/wasm/wasm.go
+++ b/wasm/wasm.go
@@ -4,6 +4,7 @@ package wasm
 import (
 	"context"
 	"fmt"
+	"io"
 	"math"
 )
 
@@ -62,6 +63,11 @@ func ValueTypeName(t ValueType) string {
 type Module interface {
 	fmt.Stringer
 
+	// Closer (Close) releases resources allocated for this Module. Using this while having outstanding function calls
+	// is safe. After calling this function, re-instantiating a module for the same name is allowed.
+	io.Closer
+	// ^^ io.Closer not due to I/O, but to allow future static analysis to catch leaks (unclosed Closers).
+
 	// Context returns any propagated context from the Runtime or a prior function call.
 	//
 	// The returned context is always non-nil; it defaults to context.Background.
@@ -87,10 +93,6 @@ type Module interface {
 
 	// ExportedGlobal a global exported from this module or nil if it wasn't.
 	ExportedGlobal(name string) Global
-
-	// Close releases resources allocated for this Module. Using this while having outstanding function calls is
-	// safe. After calling this function, re-instantiating a module for the same name is allowed.
-	Close()
 }
 
 // Function is a WebAssembly 1.0 (20191205) function exported from an instantiated module (wazero.Runtime InstantiateModule).


Test patch:
diff --git a/internal/wasm/store_test.go b/internal/wasm/store_test.go
index b3972fd9..d1776e91 100644
--- a/internal/wasm/store_test.go
+++ b/internal/wasm/store_test.go
@@ -147,15 +147,15 @@ func TestStore_CloseModule(t *testing.T) {
 			_, ok = s.modules[importingModuleName]
 			require.True(t, ok)
 
-			// Release the importing module
-			s.CloseModule(importingModuleName)
+			// Close the importing module
+			require.NoError(t, s.CloseModule(importingModuleName))
 			require.NotContains(t, s.modules, importingModuleName)
 
-			// Can re-release the importing module
-			s.CloseModule(importingModuleName)
+			// Can re-close the importing module
+			require.NoError(t, s.CloseModule(importingModuleName))
 
-			// Now we release the imported module.
-			s.CloseModule(importedModuleName)
+			// Now we close the imported module.
+			require.NoError(t, s.CloseModule(importedModuleName))
 			require.Nil(t, s.modules[importedModuleName])
 			require.NotContains(t, s.modules, importedModuleName)
 		})
@@ -206,13 +206,13 @@ func TestStore_concurrent(t *testing.T) {
 	for i := 0; i < goroutines; i++ {
 		go func(i int) {
 			defer wg.Done()
-			s.CloseModule(strconv.Itoa(i))
+			require.NoError(t, s.CloseModule(strconv.Itoa(i)))
 			require.NoError(t, err)
 		}(i)
 	}
 	wg.Wait()
 
-	s.CloseModule(hm.Name)
+	require.NoError(t, s.CloseModule(hm.Name))
 
 	// All instances are freed.
 	require.Len(t, s.modules, 0)
diff --git a/tests/engine/adhoc_test.go b/tests/engine/adhoc_test.go
index b87e4121..cbb10c96 100644
--- a/tests/engine/adhoc_test.go
+++ b/tests/engine/adhoc_test.go
@@ -236,9 +236,9 @@ func testHostFunctions(t *testing.T, newRuntimeConfig func() *wazero.RuntimeConf
 func testAdhocCloseWhileExecution(t *testing.T, newRuntimeConfig func() *wazero.RuntimeConfig) {
 	t.Run("singleton", func(t *testing.T) {
 		r := wazero.NewRuntimeWithConfig(newRuntimeConfig())
-		var moduleCloser func()
+		var moduleCloser func() error
 		_, err := r.NewModuleBuilder("host").ExportFunctions(map[string]interface{}{
-			"close_module": func() { moduleCloser() }, // Closing while executing itself.
+			"close_module": func() { _ = moduleCloser() }, // Closing while executing itself.
 		}).Instantiate()
 		require.NoError(t, err)
 
@@ -275,7 +275,7 @@ func testAdhocCloseWhileExecution(t *testing.T, newRuntimeConfig func() *wazero.
 		require.NoError(t, err)
 
 		// Closing the imported module before making call should also safe.
-		importedModule.Close()
+		require.NoError(t, importedModule.Close())
 
 		// Even we can re-enstantiate the module for the same name.
 		importedModuleNew, err := r.NewModuleBuilder("host").ExportFunctions(map[string]interface{}{
@@ -284,7 +284,7 @@ func testAdhocCloseWhileExecution(t *testing.T, newRuntimeConfig func() *wazero.
 			},
 		}).Instantiate()
 		require.NoError(t, err)
-		defer importedModuleNew.Close()
+		defer importedModuleNew.Close() // nolint
 
 		_, err = m.ExportedFunction("close_parent_before_execution").Call(nil)
 		require.NoError(t, err)