Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- **BREAKING**: `data.Setter.AddDataToContext` no longer takes a variadic.
Signature is now `AddDataToContext(ctx, data map[string]any) (context.Context, error)`.
Callers with multiple sources must merge them first.
- **BREAKING**: `Compiler.Compile` and `Loader.GetReader` now take a
`context.Context` as the first argument. `NewExecutableUnit` gains a leading
`ctx` too. The Extism compiler's `WithContext` option and the FromHTTP
loader's `GetReaderWithContext` helper are removed (now redundant).

### Deprecated
- The twelve legacy top-level constructors (`FromRisorFile`,
Expand Down
24 changes: 13 additions & 11 deletions engines/extism/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ type compileFunc func(ctx context.Context, wasmBytes []byte, opts *compile.Setti
// Compiler implements the script.Compiler interface for WASM modules
type Compiler struct {
entryPointName string
ctx context.Context
options *compile.Settings
logHandler slog.Handler
logger *slog.Logger
Expand Down Expand Up @@ -56,24 +55,25 @@ func (c *Compiler) String() string {
return "extism.Compiler"
}

// Compile implements script.Compiler
func (c *Compiler) Compile(scriptReader io.ReadCloser) (script.ExecutableContent, error) {
// Compile implements script.Compiler. Cancelling ctx halts the WASM
// compilation and the entry-point probe.
func (c *Compiler) Compile(ctx context.Context, scriptReader io.ReadCloser) (script.ExecutableContent, error) {
logger := c.logger.WithGroup("compile")

if scriptReader == nil {
return nil, ErrContentNil
}
defer func() {
if err := scriptReader.Close(); err != nil {
logger.Warn("failed to close script reader", "error", err)
}
}()

scriptBytes, err := io.ReadAll(scriptReader)
if err != nil {
return nil, fmt.Errorf("failed to read script: %w", err)
}

err = scriptReader.Close()
if err != nil {
return nil, fmt.Errorf("failed to close reader: %w", err)
}

if len(scriptBytes) == 0 {
logger.Error("Compile called with empty script")
return nil, ErrContentNil
Expand All @@ -82,7 +82,7 @@ func (c *Compiler) Compile(scriptReader io.ReadCloser) (script.ExecutableContent
logger.Debug("Starting WASM compilation", "scriptLength", len(scriptBytes))

// Compile the WASM module using the configured compile function (defaults to compile.CompileBytes)
plugin, err := c.compileFn(c.ctx, scriptBytes, c.options)
plugin, err := c.compileFn(ctx, scriptBytes, c.options)
if err != nil {
return nil, fmt.Errorf("%w: %w", ErrValidationFailed, err)
}
Expand All @@ -92,12 +92,14 @@ func (c *Compiler) Compile(scriptReader io.ReadCloser) (script.ExecutableContent
}

// Create a temporary instance to verify the entry point exists
instance, err := plugin.Instance(c.ctx, extismSDK.PluginInstanceConfig{})
instance, err := plugin.Instance(ctx, extismSDK.PluginInstanceConfig{})
if err != nil {
return nil, fmt.Errorf("%w: failed to create test instance: %w", ErrValidationFailed, err)
}
defer func() {
if err := instance.Close(c.ctx); err != nil {
// Use a cancel-immune ctx for cleanup so a cancelled compile ctx
// doesn't abort the plugin instance Close and leak resources.
if err := instance.Close(context.WithoutCancel(ctx)); err != nil {
logger.Warn("Failed to close Extism plugin instance in compiler", "error", err)
}
}()
Comment on lines 94 to 105
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Good catch — fixed in 2c024eb. The deferred instance.Close now wraps the caller ctx with context.WithoutCancel(ctx), so a cancelled compile ctx still gets a chance to release the plugin instance cleanly without the WithoutCancel-introduced detachment masking real Close failures.


Generated by Claude Code

Expand Down
37 changes: 19 additions & 18 deletions engines/extism/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser(wasmBytes)
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.NoError(t, err)
require.NotNil(t, execContent)

Expand Down Expand Up @@ -164,7 +164,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser(wasmBytes)
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.NoError(t, err)
require.NotNil(t, execContent)

Expand Down Expand Up @@ -208,7 +208,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser(wasmBytes)
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.NoError(t, err)
require.NotNil(t, execContent)

Expand All @@ -231,7 +231,7 @@ func TestCompiler_Compile(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, comp)

execContent, err := comp.Compile(nil)
execContent, err := comp.Compile(t.Context(), nil)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrContentNil)
Expand All @@ -248,7 +248,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser([]byte{})
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrContentNil)
Expand All @@ -267,7 +267,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser([]byte("not-wasm"))
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrValidationFailed)
Expand All @@ -287,7 +287,7 @@ func TestCompiler_Compile(t *testing.T) {
reader := newMockScriptReaderCloser(wasmBytes)
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrValidationFailed)
Expand All @@ -309,28 +309,29 @@ func TestCompiler_Compile_Branches(t *testing.T) {
comp := createTestCompiler(t, "main")
reader := &errReader{readErr: errors.New("read kaboom")}

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorContains(t, err, "failed to read script")
require.ErrorContains(t, err, "read kaboom")
})

t.Run("scriptReader.Close fails", func(t *testing.T) {
t.Run("scriptReader.Close error is silenced", func(t *testing.T) {
t.Parallel()
comp := createTestCompiler(t, "main")
// Read succeeds (returns EOF on first Read with empty buffer is fine for
// io.ReadAll, but we want non-empty bytes so Close error is the only failure).
// Close errors are logged via the deferred cleanup, not returned. The
// "anything" bytes still fail validation; we assert that error
// propagates (not the close error).
reader := &readCloseErr{
buf: bytes.NewReader([]byte("anything")),
closeErr: errors.New("close kaboom"),
}

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorContains(t, err, "failed to close reader")
require.ErrorContains(t, err, "close kaboom")
require.ErrorIs(t, err, ErrValidationFailed)
require.NotContains(t, err.Error(), "close kaboom")
})

t.Run("compileFn returns nil plugin without error", func(t *testing.T) {
Expand All @@ -347,7 +348,7 @@ func TestCompiler_Compile_Branches(t *testing.T) {
reader := newMockScriptReaderCloser([]byte("any-bytes"))
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrBytecodeNil)
Expand All @@ -373,7 +374,7 @@ func TestCompiler_Compile_Branches(t *testing.T) {
reader := newMockScriptReaderCloser([]byte("any-bytes"))
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrValidationFailed)
Expand Down Expand Up @@ -405,7 +406,7 @@ func TestCompiler_Compile_Branches(t *testing.T) {
reader := newMockScriptReaderCloser([]byte("any-bytes"))
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.NoError(t, err, "instance.Close error must not propagate")
require.NotNil(t, execContent)
plugin.AssertExpectations(t)
Expand Down Expand Up @@ -435,7 +436,7 @@ func TestCompiler_Compile_Branches(t *testing.T) {
reader := newMockScriptReaderCloser([]byte("any-bytes"))
reader.On("Close").Return(nil)

execContent, err := comp.Compile(reader)
execContent, err := comp.Compile(t.Context(), reader)
require.Error(t, err)
require.Nil(t, execContent)
require.ErrorIs(t, err, ErrValidationFailed)
Expand Down
22 changes: 0 additions & 22 deletions engines/extism/compiler/options.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package compiler

import (
"context"
"fmt"
"log/slog"

Expand Down Expand Up @@ -96,17 +95,6 @@ func WithHostFunctions(funcs []extismSDK.HostFunction) FunctionalOption {
}
}

// WithContext creates an option to set a custom context for the Extism compiler.
func WithContext(ctx context.Context) FunctionalOption {
return func(c *Compiler) error {
if ctx == nil {
return fmt.Errorf("context cannot be nil")
}
c.ctx = ctx
return nil
}
}

// applyDefaults sets the default values for a compiler.
func (c *Compiler) applyDefaults() {
// Set default entry point
Expand All @@ -132,11 +120,6 @@ func (c *Compiler) applyDefaults() {
// Default WASI to true (EnableWASI is a bool so we don't need to check if it's nil)
c.options.EnableWASI = true

// Default context
if c.ctx == nil {
c.ctx = context.Background()
}

// Default compile function (test seam); production path is compile.CompileBytes
if c.compileFn == nil {
c.compileFn = compile.CompileBytes
Expand Down Expand Up @@ -167,10 +150,5 @@ func (c *Compiler) validate() error {
return fmt.Errorf("runtime config cannot be nil")
}

// Context cannot be nil
if c.ctx == nil {
return fmt.Errorf("context cannot be nil")
}

return nil
}
Loading