Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make sure we instantiate non-main modules #93

Merged
merged 11 commits into from
Jan 30, 2025
42 changes: 36 additions & 6 deletions extism_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import (
"context"
"encoding/json"
"fmt"
"log"
"os"
"strings"
"sync"
"testing"
"time"

observe "github.com/dylibso/observe-sdk/go"
"github.com/dylibso/observe-sdk/go/adapter/stdout"
"github.com/stretchr/testify/assert"
Expand All @@ -13,12 +20,6 @@ import (
"github.com/tetratelabs/wazero/experimental"
"github.com/tetratelabs/wazero/experimental/logging"
"github.com/tetratelabs/wazero/sys"
"log"
"os"
"strings"
"sync"
"testing"
"time"
)

func TestWasmUrl(t *testing.T) {
Expand Down Expand Up @@ -1038,6 +1039,35 @@ func TestEnableExperimentalFeature(t *testing.T) {
}
}

func TestModuleLinking(t *testing.T) {
manifest := Manifest{
Wasm: []Wasm{
WasmFile{
Path: "wasm/lib.wasm",
Name: "lib",
},
WasmFile{
Path: "wasm/main.wasm",
Name: "main",
},
},
}

if plugin, ok := pluginInstance(t, manifest); ok {
defer plugin.Close(context.Background())

exit, output, err := plugin.Call("run_test", []byte("benjamin"))

if assertCall(t, err, exit) {
expected := "Hello, BENJAMIN"

actual := string(output)

assert.Equal(t, expected, actual)
}
}
}

func BenchmarkInitialize(b *testing.B) {
ctx := context.Background()
cache := wazero.NewCompilationCache()
Expand Down
35 changes: 25 additions & 10 deletions plugin.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is also a line:

moduleConfig = moduleConfig.WithName(strconv.Itoa(int(p.instanceCount.Add(1))))

It seems to be redundant now + p.instanceCount also can be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, i am not sure, the comment for p.instanceCount suggests otherwise. @evacchi what do you think? I moved the naming closer to the main module initialization to make it clearer that this naming is for the main module

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, you are correct, InstantiateModule comment states that module name can't be reused, this probably also means that other modules can't be instantiated multiple times?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC not with the same name, within the same runtime

Original file line number Diff line number Diff line change
@@ -1,25 +1,28 @@
// new
package extism

import (
"context"
"errors"
"fmt"
observe "github.com/dylibso/observe-sdk/go"
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
"os"
"strconv"
"strings"
"sync/atomic"
"time"

observe "github.com/dylibso/observe-sdk/go"
"github.com/tetratelabs/wazero"
"github.com/tetratelabs/wazero/api"
"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
)

type CompiledPlugin struct {
runtime wazero.Runtime
main wazero.CompiledModule
extism wazero.CompiledModule
env api.Module
modules map[string]wazero.CompiledModule

// when a module (main) is instantiated, it may have a module name that's added
// to the data section of the wasm. If this is the case, we won't be able to
Expand Down Expand Up @@ -120,6 +123,7 @@ func NewCompiledPlugin(
observeAdapter: config.ObserveAdapter,
observeOptions: config.ObserveOptions,
enableHttpResponseHeaders: config.EnableHttpResponseHeaders,
modules: make(map[string]wazero.CompiledModule),
}

if config.EnableWasi {
Expand Down Expand Up @@ -158,19 +162,18 @@ func NewCompiledPlugin(
// - If there is only one module in the manifest then that is the main module by default
// - Otherwise the last module listed is the main module

modules := map[string]wazero.CompiledModule{}
for i, wasm := range manifest.Wasm {
data, err := wasm.ToWasmData(ctx)
if err != nil {
return nil, err
}

_, mainExists := modules["main"]
_, mainExists := p.modules["main"]
mhmd-azeez marked this conversation as resolved.
Show resolved Hide resolved
if data.Name == "" || i == len(manifest.Wasm)-1 && !mainExists {
data.Name = "main"
}

_, okm := modules[data.Name]
_, okm := p.modules[data.Name]

if data.Name == "extism:host/env" || okm {
mhmd-azeez marked this conversation as resolved.
Show resolved Hide resolved
return nil, fmt.Errorf("module name collision: '%s'", data.Name)
Expand All @@ -194,7 +197,7 @@ func NewCompiledPlugin(
if data.Name == "main" {
p.main = m
} else {
modules[data.Name] = m
p.modules[data.Name] = m
}
}

Expand Down Expand Up @@ -251,8 +254,6 @@ func (p *CompiledPlugin) Instance(ctx context.Context, config PluginInstanceConf
if err != nil {
return nil, fmt.Errorf("failed to initialize Observe Adapter: %v", err)
}

trace.Finish()
Copy link
Collaborator Author

@mhmd-azeez mhmd-azeez Jan 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

calling Finish on the trace context right after creating it didn't make sense (we're storing the trace context in the plugin struct), so i removed the call

}

// Compile and instantiate the extism runtime. This runtime is stateful and needs to be
Expand All @@ -266,6 +267,20 @@ func (p *CompiledPlugin) Instance(ctx context.Context, config PluginInstanceConf
}
closers = append(closers, extism.Close)

// Instantiate all non-main modules first
instancedModules := make(map[string]api.Module)
for name, module := range p.modules {
instance, err := p.runtime.InstantiateModule(ctx, module, moduleConfig.WithName(name))
if err != nil {
for _, m := range instancedModules {
mhmd-azeez marked this conversation as resolved.
Show resolved Hide resolved
m.Close(ctx)
}
return nil, fmt.Errorf("instantiating module %s: %w", name, err)
}
instancedModules[name] = instance
closers = append(closers, instance.Close)
}

main, err := p.runtime.InstantiateModule(ctx, p.main, moduleConfig)
if err != nil {
return nil, fmt.Errorf("instantiating module: %w", err)
Expand Down
2 changes: 2 additions & 0 deletions plugins/lib/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
build:
tinygo build -target wasi -o ../../wasm/lib.wasm main.go
5 changes: 5 additions & 0 deletions plugins/lib/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module github.com/extism/extism-sdk-plugins-lib

go 1.22

require github.com/extism/go-pdk v1.0.2
2 changes: 2 additions & 0 deletions plugins/lib/go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
github.com/extism/go-pdk v1.0.2 h1:UB7oTW3tw2zoMlsUdBEDAAbhQg9OudzgNeyCwQYZ730=
github.com/extism/go-pdk v1.0.2/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4=
18 changes: 18 additions & 0 deletions plugins/lib/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package main

import (
"strings"

pdk "github.com/extism/go-pdk"
)

//go:export capitalize
func Capitalize(ptr uint64) uint64 {
mem := pdk.FindMemory(ptr)
bytes := mem.ReadBytes()
capitalized := strings.ToUpper(string(bytes))
out := pdk.AllocateString(capitalized)
return out.Offset()
}

func main() {}
2 changes: 2 additions & 0 deletions plugins/main/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
build:
tinygo build -target wasi -o ../../wasm/main.wasm main.go
5 changes: 5 additions & 0 deletions plugins/main/go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
module github.com/extism/extism-sdk-plugins-main

go 1.22

require github.com/extism/go-pdk v1.0.2
2 changes: 2 additions & 0 deletions plugins/main/go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
github.com/extism/go-pdk v1.0.2 h1:UB7oTW3tw2zoMlsUdBEDAAbhQg9OudzgNeyCwQYZ730=
github.com/extism/go-pdk v1.0.2/go.mod h1:Gz+LIU/YCKnKXhgge8yo5Yu1F/lbv7KtKFkiCSzW/P4=
24 changes: 24 additions & 0 deletions plugins/main/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package main

import (
"github.com/extism/go-pdk"
)

//go:wasm-module lib
//export capitalize
func Capitalize(offset uint64) uint64

//go:export run_test
func run_test() int32 {
name := pdk.InputString()

ptr := pdk.AllocateString(name)
capitalizedPtr := Capitalize(ptr.Offset())
capitalizedMem := pdk.FindMemory(capitalizedPtr)
capitalized := string(capitalizedMem.ReadBytes())

pdk.OutputString("Hello, " + capitalized)
return 0
}

func main() {}
Binary file added wasm/lib.wasm
Binary file not shown.
Binary file added wasm/main.wasm
Binary file not shown.
Loading