Skip to content

Commit

Permalink
fixes nil panic on close (#1286)
Browse files Browse the repository at this point in the history
The last PR over-solved the WASI close problem as it special cased both
"_start", used by WASI, and the start section (wasm level) which isn't
used by WASI. In the latter case, we ended up returning nil for both the
module *and* the error result. We should never do that.. If we coerce
exit error zero to nil, we have to return a non-nil module, even if it
is unusable as otherwise code like `defer mod.Close(ctx)` will panic.

Signed-off-by: Adrian Cole <adrian@tetrate.io>
  • Loading branch information
codefromthecrypt authored Mar 24, 2023
1 parent 451c792 commit 22f8d9d
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 8 deletions.
5 changes: 0 additions & 5 deletions runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -308,11 +308,6 @@ func (r *runtime) InstantiateModule(
if code.closeWithModule {
_ = code.Close(ctx) // don't overwrite the error
}
if se, ok := err.(*sys.ExitError); ok {
if se.ExitCode() == 0 { // Don't err on success.
err = nil
}
}
return
}

Expand Down
12 changes: 9 additions & 3 deletions runtime_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,9 @@ func TestRuntime_InstantiateModule_ExitError(t *testing.T) {
expectedErr error
}{
{
name: "start: exit code 0",
exitCode: 0,
name: "start: exit code 0",
exitCode: 0,
expectedErr: sys.NewExitError(0),
},
{
name: "start: exit code 2",
Expand Down Expand Up @@ -531,11 +532,16 @@ func TestRuntime_InstantiateModule_ExitError(t *testing.T) {
binary := binaryencoding.EncodeModule(mod)

// Instantiate the module, which calls the start function.
_, err = r.InstantiateWithConfig(testCtx, binary,
m, err := r.InstantiateWithConfig(testCtx, binary,
NewModuleConfig().WithName("call-exit"))

// Ensure the exit error propagated and didn't wrap.
require.Equal(t, tc.expectedErr, err)

// Ensure calling close again doesn't break
if err == nil {
require.NoError(t, m.Close(testCtx))
}
})
}
}
Expand Down

0 comments on commit 22f8d9d

Please sign in to comment.