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
12 changes: 11 additions & 1 deletion bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func getContextAndModuleBuilder(ctx *C.JSContext, m *C.JSModuleDef) (*Context, *
// Extract ModuleBuilder ID from private value using JS_ToInt32
var builderID C.int32_t
C.JS_ToInt32(ctx, &builderID, privateValue)
C.JS_FreeValue(ctx, privateValue)

goCtx, builderInterface, err := getContextAndObject(ctx, C.int(builderID), errFunctionNotFound)
if err != nil {
Expand Down Expand Up @@ -487,8 +488,16 @@ func goModuleInitProxy(ctx *C.JSContext, m *C.JSModuleDef) C.int {
// Step 2: Set all export values using JS_SetModuleExport
for _, export := range builder.exports {
exportName := C.CString(export.Name)
C.JS_SetModuleExport(ctx, m, exportName, export.Value.ref)
// JS_SetModuleExport takes ownership of the JSValue (it will free it on failure).
// To prevent Go-side double free (issue #688), invalidate the Go Value after
// handing it off so later export.Value.Free() becomes a no-op.
val := export.Value.ref
rc := C.JS_SetModuleExport(ctx, m, exportName, val)
export.Value.ref = C.JS_NewUndefined()
C.free(unsafe.Pointer(exportName))
if rc < 0 {
return throwModuleError(ctx, fmt.Errorf("failed to set module export: %s", export.Name))
}
}

// Step 3: Clean up HandleStore reference
Expand All @@ -497,6 +506,7 @@ func goModuleInitProxy(ctx *C.JSContext, m *C.JSModuleDef) C.int {
if C.JS_ToInt32(ctx, &builderID, privateValue) >= 0 {
goCtx.handleStore.Delete(int32(builderID))
}
C.JS_FreeValue(ctx, privateValue)

return C.int(0)
}
68 changes: 68 additions & 0 deletions module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -350,4 +350,72 @@ func TestModuleBuilder_ErrorBranches(t *testing.T) {
err = ctx.Exception()
require.Contains(t, err.Error(), "Function not found")
})

t.Run("ModuleInitSetModuleExportError", func(t *testing.T) {
fooFunc := ctx.NewFunction(func(ctx *Context, this *Value, args []*Value) *Value {
return ctx.NewUndefined()
})
defer fooFunc.Free()

module := NewModuleBuilder("error-test-3").Export("foo", fooFunc)
err := module.Build(ctx)
require.NoError(t, err)

// Force a mismatch between declared exports ("foo") and init-time exports.
// Build() declares exports based on the current builder.exports. Module init happens
// later during import; by mutating the builder, JS_SetModuleExport will fail.
require.GreaterOrEqual(t, len(module.exports), 1)
module.exports[0].Name = "bar"

result := ctx.Eval(`import('error-test-3')`, EvalAwait(true))
defer result.Free()

require.True(t, result.IsException())
err = ctx.Exception()
require.Contains(t, err.Error(), "failed to set module export")
})
}

// =============================================================================
// MINIMAL REPRO TEST (ISSUE #688)
// =============================================================================

func TestModuleBuilder_RuntimeClosePanic_Minimal(t *testing.T) {
// Regression stress test for issue #688. Historically, closing the runtime after
// importing a native module could randomly trigger a QuickJS abort in rt.Close().
//
// If QuickJS triggers an assertion/abort during rt.Close(), `go test` will terminate.
const attempts = 50
for i := 1; i <= attempts; i++ {
t.Run(fmt.Sprintf("attempt_%d", i), func(t *testing.T) {
// Use an inner scope so defers run per-iteration.
rt := NewRuntime(WithModuleImport(true))
defer rt.Close()

ctx := rt.NewContext()
defer ctx.Close()

fooFunc := ctx.NewFunction(func(ctx *Context, this *Value, args []*Value) *Value {
fmt.Println("foo")
return ctx.NewUndefined()
})
defer fooFunc.Free()

module := NewModuleBuilder("testmodule")
module.Export("foo", fooFunc)
require.NoError(t, module.Build(ctx))

script := `
import * as testmodule from "testmodule";

testmodule.foo();
`

result := ctx.Eval(script, EvalFlagStrict(true), EvalAwait(true))
defer result.Free()
if result.IsException() {
panic(ctx.Exception())
}
})
}
}