Support registering multiple instances of anonymous modules (#1259)

Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
Signed-off-by: Clifton Kaznocha <ckaznocha@users.noreply.github.com>
Co-authored-by: Clifton Kaznocha <ckaznocha@users.noreply.github.com>

internal/integration_test/bench/bench_test.go

@@ -6,8 +6,6 @@ import (
 	_ "embed"
 	"fmt"
 	"runtime"
-	"strconv"
-	"sync"
 	"testing"
 
 	"github.com/tetratelabs/wazero"
@@ -118,22 +116,21 @@ func runInitializationConcurrentBench(b *testing.B, r wazero.Runtime) {
 	defer compiled.Close(testCtx)
 	// Configure with real sources to avoid performance hit initializing fake ones. These sources are not used
 	// in the benchmark.
-	config := wazero.NewModuleConfig().WithSysNanotime().WithSysWalltime().WithRandSource(rand.Reader)
-	wg := &sync.WaitGroup{}
-	wg.Add(b.N)
-	b.ResetTimer()
-	for i := 0; i < b.N; i++ {
-		go func(name string) {
-			m, err := r.InstantiateModule(testCtx, compiled, config.WithName(name))
+	config := wazero.NewModuleConfig().
+		WithSysNanotime().
+		WithSysWalltime().
+		WithRandSource(rand.Reader).
+		WithName("")
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			m, err := r.InstantiateModule(testCtx, compiled, config)
 			if err != nil {
 				b.Error(err)
+			} else {
+				m.Close(testCtx)
 			}
-			m.Close(testCtx)
-			wg.Done()
-		}(strconv.Itoa(i))
-	}
-	wg.Wait()
-	b.StopTimer()
+		}
+	})
 }
 
 func runAllInvocationBenches(b *testing.B, m api.Module) {

internal/wasm/call_context.go

@@ -134,7 +134,7 @@ func (m *CallContext) CloseWithExitCode(ctx context.Context, exitCode uint32) (e
 	if !m.setExitCode(exitCode) {
 		return nil // not an error to have already closed
 	}
-	_ = m.s.deleteModule(m.Name())
+	_ = m.s.deleteModule(m.module.moduleListNode)
 	return m.ensureResourcesClosed(ctx)
 }
 

internal/wasm/call_context_test.go

@@ -44,11 +44,14 @@ func TestCallContext_String(t *testing.T) {
 
 			require.NoError(t, err)
 			require.Equal(t, tc.expected, m.String())
-			sm := s.Module(m.Name())
-			if sm != nil {
-				require.Equal(t, tc.expected, s.Module(m.Name()).String())
-			} else {
-				require.Zero(t, len(m.Name()))
+
+			if name := m.Name(); name != "" {
+				sm := s.Module(m.Name())
+				if sm != nil {
+					require.Equal(t, tc.expected, s.Module(m.Name()).String())
+				} else {
+					require.Zero(t, len(m.Name()))
+				}
 			}
 		})
 	}

internal/wasm/store.go

@@ -83,6 +83,8 @@ type (
 		// ElementInstances holds the element instance, and each holds the references to either functions
 		// or external objects (unimplemented).
 		ElementInstances []ElementInstance
+
+		moduleListNode *moduleListNode
 	}
 
 	// DataInstance holds bytes corresponding to the data segment in a module.
@@ -274,24 +276,35 @@ func (s *Store) Instantiate(
 		return nil, err
 	}
 
-	// Write-Lock the store and claim the name of the current module.
-	if err = s.requireModuleName(name); err != nil {
-		return nil, err
+	var listNode *moduleListNode
+	if name == "" {
+		listNode = s.registerAnonymous()
+	} else {
+		// Write-Lock the store and claim the name of the current module.
+		listNode, err = s.requireModuleName(name)
+		if err != nil {
+			return nil, err
+		}
 	}
 
 	// Instantiate the module and add it to the store so that other modules can import it.
-	if callCtx, err := s.instantiate(ctx, module, name, sys, importedModules, typeIDs); err != nil {
-		_ = s.deleteModule(name)
+	callCtx, err := s.instantiate(ctx, module, name, sys, importedModules, typeIDs)
+	if err != nil {
+		_ = s.deleteModule(listNode)
 		return nil, err
-	} else {
+	}
+
+	callCtx.module.moduleListNode = listNode
+
+	if name != "" {
 		// Now that the instantiation is complete without error, add it.
 		// This makes the module visible for import, and ensures it is closed when the store is.
 		if err := s.setModule(callCtx.module); err != nil {
 			callCtx.Close(ctx)
 			return nil, err
 		}
-		return callCtx, nil
 	}
+	return callCtx, nil
 }
 
 func (s *Store) instantiate(

internal/wasm/store_module_list.go

@@ -17,6 +17,7 @@ type moduleListNode struct {
 func (s *Store) setModule(m *ModuleInstance) error {
 	s.mux.Lock()
 	defer s.mux.Unlock()
+
 	node, ok := s.nameToNode[m.Name]
 	if !ok {
 		return fmt.Errorf("module[%s] name has not been required", m.Name)
@@ -27,24 +28,32 @@ func (s *Store) setModule(m *ModuleInstance) error {
 }
 
 // deleteModule makes the moduleName available for instantiation again.
-func (s *Store) deleteModule(moduleName string) error {
-	s.mux.Lock()
-	defer s.mux.Unlock()
-	node, ok := s.nameToNode[moduleName]
-	if !ok {
+func (s *Store) deleteModule(node *moduleListNode) error {
+	if node == nil {
 		return nil
 	}
 
+	s.mux.Lock()
+	defer s.mux.Unlock()
+
 	// remove this module name
 	if node.prev != nil {
 		node.prev.next = node.next
-	} else {
-		s.moduleList = node.next
 	}
 	if node.next != nil {
 		node.next.prev = node.prev
 	}
-	delete(s.nameToNode, moduleName)
+	if s.moduleList == node {
+		s.moduleList = node.next
+	}
+	// clear the node state so it does not enter any other branch
+	// on subsequent calls to deleteModule
+	node.prev = nil
+	node.next = nil
+
+	if node.name != "" {
+		delete(s.nameToNode, node.name)
+	}
 	return nil
 }
 
@@ -83,24 +92,39 @@ func (s *Store) requireModules(moduleNames map[string]struct{}) (map[string]*Mod
 
 // requireModuleName is a pre-flight check to reserve a module.
 // This must be reverted on error with deleteModule if initialization fails.
-func (s *Store) requireModuleName(moduleName string) error {
+func (s *Store) requireModuleName(moduleName string) (*moduleListNode, error) {
+	node := &moduleListNode{name: moduleName}
+
 	s.mux.Lock()
 	defer s.mux.Unlock()
 	if _, ok := s.nameToNode[moduleName]; ok {
-		return fmt.Errorf("module[%s] has already been instantiated", moduleName)
+		return nil, fmt.Errorf("module[%s] has already been instantiated", moduleName)
 	}
 
 	// add the newest node to the moduleNamesList as the head.
-	node := &moduleListNode{
-		name: moduleName,
-		next: s.moduleList,
-	}
+	node.next = s.moduleList
 	if node.next != nil {
 		node.next.prev = node
 	}
 	s.moduleList = node
 	s.nameToNode[moduleName] = node
-	return nil
+	return node, nil
+}
+
+func (s *Store) registerAnonymous() *moduleListNode {
+	node := &moduleListNode{name: ""}
+
+	s.mux.Lock()
+	defer s.mux.Unlock()
+
+	// add the newest node to the moduleNamesList as the head.
+	node.next = s.moduleList
+	if node.next != nil {
+		node.next.prev = node
+	}
+	s.moduleList = node
+
+	return node
 }
 
 // AliasModule aliases the instantiated module named `src` as `dst`.

internal/wasm/store_module_list_test.go

@@ -52,7 +52,7 @@ func TestStore_deleteModule(t *testing.T) {
 	s, m1, m2 := newTestStore()
 
 	t.Run("delete one module", func(t *testing.T) {
-		require.NoError(t, s.deleteModule(m2.Name))
+		require.NoError(t, s.deleteModule(m2.moduleListNode))
 
 		// Leaves the other module alone
 		m1Node := &moduleListNode{name: m1.Name, module: m1}
@@ -61,11 +61,11 @@ func TestStore_deleteModule(t *testing.T) {
 	})
 
 	t.Run("ok if missing", func(t *testing.T) {
-		require.NoError(t, s.deleteModule(m2.Name))
+		require.NoError(t, s.deleteModule(m2.moduleListNode))
 	})
 
 	t.Run("delete last module", func(t *testing.T) {
-		require.NoError(t, s.deleteModule(m1.Name))
+		require.NoError(t, s.deleteModule(m1.moduleListNode))
 
 		require.Zero(t, len(s.nameToNode))
 		require.Nil(t, s.moduleList)
@@ -129,7 +129,7 @@ func TestStore_requireModuleName(t *testing.T) {
 	s := newStore()
 
 	t.Run("first", func(t *testing.T) {
-		err := s.requireModuleName("m1")
+		_, err := s.requireModuleName("m1")
 		require.NoError(t, err)
 
 		// Ensure it adds the module name, and doesn't impact the module list.
@@ -137,7 +137,7 @@ func TestStore_requireModuleName(t *testing.T) {
 		require.Equal(t, map[string]*moduleListNode{"m1": {name: "m1"}}, s.nameToNode)
 	})
 	t.Run("second", func(t *testing.T) {
-		err := s.requireModuleName("m2")
+		_, err := s.requireModuleName("m2")
 		require.NoError(t, err)
 		m2Node := &moduleListNode{name: "m2"}
 		m1Node := &moduleListNode{name: "m1", prev: m2Node}
@@ -148,7 +148,7 @@ func TestStore_requireModuleName(t *testing.T) {
 		require.Equal(t, map[string]*moduleListNode{"m1": m1Node, "m2": m2Node}, s.nameToNode)
 	})
 	t.Run("existing", func(t *testing.T) {
-		err := s.requireModuleName("m2")
+		_, err := s.requireModuleName("m2")
 		require.EqualError(t, err, "module[m2] has already been instantiated")
 	})
 }
@@ -194,5 +194,8 @@ func newTestStore() (*Store, *ModuleInstance, *ModuleInstance) {
 	node1.prev = node2
 	s.nameToNode = map[string]*moduleListNode{m1.Name: node1, m2.Name: node2}
 	s.moduleList = node2
+
+	m1.moduleListNode = node1
+	m2.moduleListNode = node2
 	return s, m1, m2
 }