Commit c2e2296d authored by Richard Musiol's avatar Richard Musiol Committed by Richard Musiol

syscall/js: handle interleaved functions correctly

Because of concurrent goroutines it is possible for multiple event
handlers to return at the same time. This was not properly supported
and caused the wrong goroutine to continue, which in turn caused
memory corruption.

This change adds a stack of events so it is always clear which is the
innermost event that needs to return next.

Fixes #35256

Change-Id: Ia527da3b91673bc14e84174cdc407f5c9d5a3d09
Reviewed-on: https://go-review.googlesource.com/c/go/+/204662
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: default avatarCherry Zhang <cherryyz@google.com>
parent 688aa748
......@@ -146,9 +146,18 @@ func checkTimeouts() {
}
}
var isHandlingEvent = false
var nextEventIsAsync = false
var returnedEventHandler *g
// events is a stack of calls from JavaScript into Go.
var events []*event
type event struct {
// g was the active goroutine when the call from JavaScript occurred.
// It needs to be active when returning to JavaScript.
gp *g
// returned reports whether the event handler has returned.
// When all goroutines are idle and the event handler has returned,
// then g gets resumed and returns the execution to JavaScript.
returned bool
}
// The timeout event started by beforeIdle.
var idleID int32
......@@ -170,18 +179,24 @@ func beforeIdle(delay int64) bool {
}
idleID = scheduleTimeoutEvent(delay)
}
if !isHandlingEvent {
nextEventIsAsync = true
pause(getcallersp() - 16)
if len(events) == 0 {
go handleAsyncEvent()
return true
}
if returnedEventHandler != nil {
goready(returnedEventHandler, 1)
e := events[len(events)-1]
if e.returned {
goready(e.gp, 1)
return true
}
return false
}
func handleAsyncEvent() {
pause(getcallersp() - 16)
}
// clearIdleID clears our record of the timeout started by beforeIdle.
func clearIdleID() {
if idleID != 0 {
......@@ -200,40 +215,32 @@ func scheduleTimeoutEvent(ms int64) int32
// clearTimeoutEvent clears a timeout event scheduled by scheduleTimeoutEvent.
func clearTimeoutEvent(id int32)
// handleEvent gets invoked on a call from JavaScript into Go. It calls the event handler of the syscall/js package
// and then parks the handler goroutine to allow other goroutines to run before giving execution back to JavaScript.
// When no other goroutine is awake any more, beforeIdle resumes the handler goroutine. Now that the same goroutine
// is running as was running when the call came in from JavaScript, execution can be safely passed back to JavaScript.
func handleEvent() {
if nextEventIsAsync {
nextEventIsAsync = false
checkTimeouts()
go handleAsyncEvent()
return
e := &event{
gp: getg(),
returned: false,
}
prevIsHandlingEvent := isHandlingEvent
isHandlingEvent = true
prevReturnedEventHandler := returnedEventHandler
returnedEventHandler = nil
events = append(events, e)
eventHandler()
clearIdleID()
// wait until all goroutines are idle
returnedEventHandler = getg()
e.returned = true
gopark(nil, nil, waitReasonZero, traceEvNone, 1)
isHandlingEvent = prevIsHandlingEvent
returnedEventHandler = prevReturnedEventHandler
events[len(events)-1] = nil
events = events[:len(events)-1]
// return execution to JavaScript
pause(getcallersp() - 16)
}
func handleAsyncEvent() {
isHandlingEvent = true
eventHandler()
clearIdleID()
isHandlingEvent = false
}
var eventHandler func()
//go:linkname setEventHandler syscall/js.setEventHandler
......
......@@ -419,6 +419,25 @@ func TestInvokeFunction(t *testing.T) {
}
}
func TestInterleavedFunctions(t *testing.T) {
c1 := make(chan struct{})
c2 := make(chan struct{})
js.Global().Get("setTimeout").Invoke(js.FuncOf(func(this js.Value, args []js.Value) interface{} {
c1 <- struct{}{}
<-c2
return nil
}), 0)
<-c1
c2 <- struct{}{}
// this goroutine is running, but the callback of setTimeout did not return yet, invoke another function now
f := js.FuncOf(func(this js.Value, args []js.Value) interface{} {
return nil
})
f.Invoke()
}
func ExampleFuncOf() {
var cb js.Func
cb = js.FuncOf(func(this js.Value, args []js.Value) interface{} {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment