From c2e2296dd7dea68de4a45d8dc82b1920130eb74a Mon Sep 17 00:00:00 2001
From: Richard Musiol <mail@richard-musiol.de>
Date: Sat, 2 Nov 2019 21:20:24 +0100
Subject: [PATCH] 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>
---
 src/runtime/lock_js.go    | 63 ++++++++++++++++++++++-----------------
 src/syscall/js/js_test.go | 19 ++++++++++++
 2 files changed, 54 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
diff --git a/src/syscall/js/js_test.go b/src/syscall/js/js_test.go
index b5d267c03c..fea4c135af 100644
--- 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{} {
-- 
2.50.0