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>

1 files changed, 35 insertions, 28 deletions

diff --git a/src/runtime/lock_js.go b/src/runtime/lock_js.go
index df52ea04fd..3168c86d8a 100644
--- 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