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

engine: call FunctionListener.Abort in correct order #1501

Merged
merged 1 commit into from
Jun 2, 2023
Merged

engine: call FunctionListener.Abort in correct order #1501

merged 1 commit into from
Jun 2, 2023

Conversation

chriso
Copy link
Contributor

@chriso chriso commented Jun 2, 2023

I have an experimental.FunctionListener like so:

type functionListener struct {
	depth int
}

func (f *functionListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
	fmt.Println(strings.Repeat(" ", f.depth), ">", def.DebugName())
	f.depth++
}

func (f *functionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
	f.depth--
	fmt.Println(strings.Repeat(" ", f.depth), "<", def.DebugName())
}

func (f *functionListener) Abort(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error) {
	f.depth--
	fmt.Println(strings.Repeat(" ", f.depth), "<", def.DebugName(), ":", err)
}

I have a simple program that calls proc_exit:

// GOOS=wasip1 GOARCH=wasm gotip build -o exit.wasm exit.go
package main

import "os"

func main() {
	os.Exit(0)
}

If I run the module with wazero, and enable the FunctionListener, I see the following:

 > ._rt0_wasm_wasip1
  > .runtime.rt0_go
   > .runtime.check
    > .runtime_internal_atomic.Cas
...<snip>...
      > .os.runtime_beforeExit
       > .runtime.runExitHooks
       < .runtime.runExitHooks
      < .os.runtime_beforeExit
      > .syscall.Exit
       > .runtime.exit
        > wasi_snapshot_preview1.proc_exit
        < ._rt0_wasm_wasip1 : module closed with exit_code(0)
       < .wasm_pc_f_loop : module closed with exit_code(0)
      < .runtime.main : module closed with exit_code(0)
     < .main.main : module closed with exit_code(0)
    < .os.Exit : module closed with exit_code(0)
   < .syscall.Exit : module closed with exit_code(0)
  < .runtime.exit : module closed with exit_code(0)
 < wasi_snapshot_preview1.proc_exit : module closed with exit_code(0)

The same is true for both the compiler and interpreter.

This PR fixes the issue by reversing the order in which function listeners and their Abort methods are called. I now see:

      > .os.runtime_beforeExit
       > .runtime.runExitHooks
       < .runtime.runExitHooks
      < .os.runtime_beforeExit
      > .syscall.Exit
       > .runtime.exit
        > wasi_snapshot_preview1.proc_exit
        < wasi_snapshot_preview1.proc_exit : module closed with exit_code(0)
       < .runtime.exit : module closed with exit_code(0)
      < .syscall.Exit : module closed with exit_code(0)
     < .os.Exit : module closed with exit_code(0)
    < .main.main : module closed with exit_code(0)
   < .runtime.main : module closed with exit_code(0)
  < .wasm_pc_f_loop : module closed with exit_code(0)
 < ._rt0_wasm_wasip1 : module closed with exit_code(0)
For reference, here's the script I used to generate the output:
package main

import (
	"context"
	"crypto/rand"
	"fmt"
	"os"
	"strings"

	"github.com/tetratelabs/wazero"
	"github.com/tetratelabs/wazero/api"
	"github.com/tetratelabs/wazero/experimental"
	"github.com/tetratelabs/wazero/imports/wasi_snapshot_preview1"
)

func main() {
	wasmFile := os.Args[1]
	wasmCode, err := os.ReadFile(wasmFile)
	if err != nil {
		panic(err)
	}

	ctx := context.Background()
	runtime := wazero.NewRuntimeWithConfig(ctx,
		wazero.NewRuntimeConfigCompiler())  // or wazero.NewRuntimeConfigInterpreter()
	defer runtime.Close(ctx)

	listener := &functionListener{}

	ctx = context.WithValue(ctx,
		experimental.FunctionListenerFactoryKey{},
		experimental.FunctionListenerFactoryFunc(func(_ api.FunctionDefinition) experimental.FunctionListener {
			return listener
		}))

	wasi_snapshot_preview1.MustInstantiate(ctx, runtime)

	wasmModule, err := runtime.CompileModule(ctx, wasmCode)
	if err != nil {
		panic(err)
	}
	defer wasmModule.Close(ctx)

	config := wazero.NewModuleConfig().
		WithStdout(os.Stdout).
		WithStderr(os.Stderr).
		WithStdin(os.Stdin).
		WithRandSource(rand.Reader).
		WithSysNanosleep().
		WithSysNanotime().
		WithSysWalltime().
		WithArgs()

	instance, err := runtime.InstantiateWithConfig(ctx, wasmCode, config)
	if err != nil {
		panic(err)
	}
	instance.Close(ctx)
}

type functionListener struct {
	depth int
}

func (f *functionListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
	fmt.Println(strings.Repeat(" ", f.depth), ">", def.DebugName())
	f.depth++
}

func (f *functionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
	f.depth--
	fmt.Println(strings.Repeat(" ", f.depth), "<", def.DebugName())
}

func (f *functionListener) Abort(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error) {
	f.depth--
	fmt.Println(strings.Repeat(" ", f.depth), "<", def.DebugName(), ":", err)
}

Signed-off-by: Chris O'Hara <cohara87@gmail.com>
Copy link
Contributor

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

good eye. also thanks for the super rigorous PR desc!

@achille-roussel achille-roussel merged commit 50394cc into tetratelabs:main Jun 2, 2023
@chriso chriso deleted the abort-order branch June 2, 2023 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants