quickjs-go icon indicating copy to clipboard operation
quickjs-go copied to clipboard

Type issue with interrupt handler

Open webmaster128 opened this issue 9 months ago • 1 comments

We have an application where we set the interrupt handler. When running tests, every now and then the following panic pops up:

interface conversion: interface {} is func(*quickjs.Context, quickjs.Value, []quickjs.Value) quickjs.Value, not quickjs.InterruptHandler

Stack is something like

panic: interface conversion: interface {} is func(*quickjs.Context, quickjs.Value, []quickjs.Value) quickjs.Value, not quickjs.InterruptHandler [recovered]
        panic: interface conversion: interface {} is func(*quickjs.Context, quickjs.Value, []quickjs.Value) quickjs.Value, not quickjs.InterruptHandler

goroutine 8 [running, locked to thread]:
testing.tRunner.func1.2({0x1023e2440, 0x1400067f4a0})
        /opt/homebrew/Cellar/go/1.24.2/libexec/src/testing/testing.go:1734 +0x1ac
testing.tRunner.func1()
        /opt/homebrew/Cellar/go/1.24.2/libexec/src/testing/testing.go:1737 +0x334
panic({0x1023e2440?, 0x1400067f4a0?})
        /opt/homebrew/Cellar/go/1.24.2/libexec/src/runtime/panic.go:792 +0x124
github.com/buke/quickjs-go.goInterruptHandler(0x140007f8b28?, 0x1009f45dc?)
        /[me]/go/pkg/mod/github.com/buke/[email protected]/bridge.go:71 +0x58
github.com/buke/quickjs-go._Cfunc_JS_Call(0x154765b40, {{0x50, 0x31, 0x78, 0x54, 0x1, 0x0, 0x0, 0x0}, 0xffffffffffffffff}, ...)
        _cgo_gotypes.go:291 +0x34
github.com/buke/quickjs-go.Value.Call.func2(...)
        /[me]/go/pkg/mod/github.com/buke/[email protected]/value.go:254
github.com/buke/quickjs-go.Value.Call({0x140008dd710, {{0xe0, 0x6a, 0x76, 0x54, 0x1, 0x0, 0x0, 0x0}, 0xffffffffffffffff}}, ...)
        /[me]/go/pkg/mod/github.com/buke/[email protected]/value.go:254 +0x388
[...]

I don't know where this func(*quickjs.Context, quickjs.Value, []quickjs.Value) quickjs.Value is coming from. The interrupt handler has certainly the correct signature.

However, looking into func (ctx *Context) SetInterruptHandler(handler InterruptHandler) I wonder why unsafe.Pointer(&handlerArgs) is safe. Isn't https://github.com/buke/quickjs-go/blob/v0.4.15/context.go#L217-L220 turning the shortly living handlerArgs := C.handlerArgs into a long living pointer? Can't Go just move handlerArgs or delete it?

webmaster128 avatar Apr 15 '25 19:04 webmaster128

It seems like this fixes the problem:

diff --git a/context.go b/context.go
index 0e5e5fc..e246a57 100644
--- a/context.go
+++ b/context.go
@@ -3,6 +3,7 @@ package quickjs
 import (
 	"fmt"
 	"os"
+	"runtime"
 	"runtime/cgo"
 	"unsafe"
 )
@@ -20,6 +21,7 @@ type Context struct {
 	globals    *Value
 	proxy      *Value
 	asyncProxy *Value
+	pinner     runtime.Pinner
 }
 
 // Runtime returns the runtime of the context.
@@ -42,6 +44,8 @@ func (ctx *Context) Close() {
 	}
 
 	C.JS_FreeContext(ctx.ref)
+
+	ctx.pinner.Unpin()
 }
 
 // Null return a null value.
@@ -214,10 +218,14 @@ type InterruptHandler func() int
 
 // SetInterruptHandler sets a interrupt handler.
 func (ctx *Context) SetInterruptHandler(handler InterruptHandler) {
-	handlerArgs := C.handlerArgs{
+	handlerArgsPtr := &C.handlerArgs{
 		fn: (C.uintptr_t)(cgo.NewHandle(handler)),
 	}
-	C.SetInterruptHandler(ctx.runtime.ref, unsafe.Pointer(&handlerArgs))
+
+	// Ensure the C.handlerArgs instance is never moved to a different place or GCed.
+	ctx.pinner.Pin(handlerArgsPtr)
+
+	C.SetInterruptHandler(ctx.runtime.ref, unsafe.Pointer(handlerArgsPtr))
 }
 
 // Atom returns a new Atom value with given string.

webmaster128 avatar Apr 15 '25 22:04 webmaster128