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: Cherry Zhang <cherryyz@google.com>

diff --git a/src/runtime/lock_js.go b/src/runtime/lock_js.go
index df52ea04fd24893df997d5b2b1b415ca9f54e066..3168c86d8a029f528e3604aef23e26bf6bbd3d74 100644 (file)
--- a/src/runtime/lock_js.go
+++ b/src/runtime/lock_js.go
@@ -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

diff --git a/src/syscall/js/js_test.go b/src/syscall/js/js_test.go
index b5d267c03c4cd5ceef587d9d31e34c3cbddc374b..fea4c135afc85abec0e2178efb821cfc076f3009 100644 (file)
--- a/src/syscall/js/js_test.go
+++ b/src/syscall/js/js_test.go
@@ -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{} {