add __xgo_link_is_system_stack() to avoid lock/unlock incorrectly, see …

…#96
xhd2015 · May 8, 2024 · 83fdf34 · 83fdf34
1 parent 1e7392b
commit 83fdf34
Show file tree

Hide file tree

Showing 12 changed files with 114 additions and 13 deletions.
diff --git a/cmd/xgo/patch/runtime_def_gen.go b/cmd/xgo/patch/runtime_def_gen.go
diff --git a/cmd/xgo/runtime_gen/core/version.go b/cmd/xgo/runtime_gen/core/version.go
@@ -6,9 +6,9 @@ import (
 	"os"
 )
 
-const VERSION = "1.0.29"
-const REVISION = "81a667d545fbd097e4403650a69af2c8477814fc+1"
-const NUMBER = 206
+const VERSION = "1.0.30"
+const REVISION = "a6ed1f5d4adc882e06ad3da0530ed5e3733c6169+1"
+const NUMBER = 208
 
 // these fields will be filled by compiler
 const XGO_VERSION = ""

diff --git a/cmd/xgo/runtime_gen/tls/hook.go b/cmd/xgo/runtime_gen/tls/hook.go
@@ -19,18 +19,40 @@ func __xgo_link_on_goexit(fn func()) {
 	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_on_goexit(requires xgo).")
 }
 
+func __xgo_link_is_system_stack() bool {
+	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_is_system_stack(requires xgo).")
+	return false
+}
+
 func init() {
 	__xgo_link_on_goexit(func() {
 		// clear when exit
 		g := uintptr(__xgo_link_getcurg())
-		mut.Lock()
+
+		// see https://github.com/xhd2015/xgo/issues/96
+		sysStack := __xgo_link_is_system_stack()
+		if !sysStack {
+			mut.Lock()
+		}
+
 		localKeys := keys
-		mut.Unlock()
+		if !sysStack {
+			mut.Unlock()
+		}
+
+		// NOTE: we must delete entries, otherwise they would
+		// cause memory leak
 		for _, loc := range localKeys {
 			loc.store.Delete(g)
 		}
 	})
+
 	__xgo_link_on_gonewproc(func(newg uintptr) {
+		// cannot lock/unlock on sys stack
+		if __xgo_link_is_system_stack() && false {
+			return
+		}
+
 		// inherit when new goroutine
 		g := uintptr(__xgo_link_getcurg())
 		mut.Lock()

diff --git a/cmd/xgo/runtime_gen/trap/trap.go b/cmd/xgo/runtime_gen/trap/trap.go
@@ -13,6 +13,11 @@ import (
 
 var setupOnce sync.Once
 
+func __xgo_link_is_system_stack() bool {
+	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_is_system_stack(requires xgo).")
+	return false
+}
+
 func ensureTrapInstall() {
 	setupOnce.Do(func() {
 		// set trap once needed, no matter it
@@ -37,6 +42,10 @@ func init() {
 		if isByPassing() {
 			return
 		}
+		if __xgo_link_is_system_stack() {
+			// cannot lock/unlock on sys stack
+			return
+		}
 		local := getLocalInterceptorList()
 		if !local.hasAny() {
 			return

diff --git a/cmd/xgo/version.go b/cmd/xgo/version.go
@@ -2,9 +2,9 @@ package main
 
 import "fmt"
 
-const VERSION = "1.0.29"
-const REVISION = "81a667d545fbd097e4403650a69af2c8477814fc+1"
-const NUMBER = 206
+const VERSION = "1.0.30"
+const REVISION = "a6ed1f5d4adc882e06ad3da0530ed5e3733c6169+1"
+const NUMBER = 208
 
 func getRevision() string {
 	revSuffix := ""

diff --git a/patch/link_name.go b/patch/link_name.go
@@ -31,6 +31,7 @@ const reflectSetImpl = "__xgo_set_all_method_by_name_impl"
 
 var linkMap = map[string]string{
 	"__xgo_link_getcurg":                      "__xgo_getcurg",
+	"__xgo_link_is_system_stack":              "__xgo_is_system_stack",
 	XgoLinkSetTrap:                            setTrap,
 	XgoLinkSetTrapVar:                         setTrapVar,
 	xgo_syntax.XgoLinkTrapForGenerated:        XgoTrapForGenerated,

diff --git a/patch/trap_runtime/xgo_trap.go b/patch/trap_runtime/xgo_trap.go
@@ -13,6 +13,12 @@ import (
 // see: https://github.com/golang/go/blob/master/src/runtime/HACKING.md
 func __xgo_getcurg() unsafe.Pointer { return unsafe.Pointer(getg().m.curg) }
 
+// used to avoid https://github.com/xhd2015/xgo/issues/96
+func __xgo_is_system_stack() bool {
+	gp := getg()
+	return gp != gp.m.curg
+}
+
 // exported so other func can call it
 var __xgo_trap_impl func(pkgPath string, identityName string, generic bool, funcPC uintptr, recv interface{}, args []interface{}, results []interface{}) (func(), bool)
 

diff --git a/runtime/core/version.go b/runtime/core/version.go
@@ -6,9 +6,9 @@ import (
 	"os"
 )
 
-const VERSION = "1.0.29"
-const REVISION = "81a667d545fbd097e4403650a69af2c8477814fc+1"
-const NUMBER = 206
+const VERSION = "1.0.30"
+const REVISION = "a6ed1f5d4adc882e06ad3da0530ed5e3733c6169+1"
+const NUMBER = 208
 
 // these fields will be filled by compiler
 const XGO_VERSION = ""

diff --git a/runtime/tls/tls_test.go → runtime/test/tls/tls_test.go b/runtime/tls/tls_test.go → runtime/test/tls/tls_test.go
@@ -1,7 +1,9 @@
 package tls_test
 
 import (
+	"fmt"
 	"testing"
+	"time"
 
 	"github.com/xhd2015/xgo/runtime/tls"
 )
@@ -54,3 +56,30 @@ func TestInerhitLocal(t *testing.T) {
 		t.Fatalf("expect current goroutine get %d, actual: %d", 1, i)
 	}
 }
+
+// see https://github.com/xhd2015/xgo/issues/96
+func TestTimeAfterShouldNotInherit(t *testing.T) {
+	b.Set(1)
+	goDone := make(chan struct{})
+	go func() {
+		defer close(goDone)
+		v, ok := b.GetOK()
+		if !ok {
+			panic("expect inherited in new proc")
+		}
+		if v.(int) != 1 {
+			panic(fmt.Errorf("expect inherited to be: %d, actual: %v", 1, v))
+		}
+	}()
+	<-goDone
+
+	timerDone := make(chan struct{})
+	time.AfterFunc(10*time.Millisecond, func() {
+		defer close(timerDone)
+		_, ok := b.GetOK()
+		if ok {
+			t.Fatalf("expect not inherited in timer")
+		}
+	})
+	<-timerDone
+}
diff --git a/runtime/tls/hook.go b/runtime/tls/hook.go
@@ -19,18 +19,40 @@ func __xgo_link_on_goexit(fn func()) {
 	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_on_goexit(requires xgo).")
 }
 
+func __xgo_link_is_system_stack() bool {
+	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_is_system_stack(requires xgo).")
+	return false
+}
+
 func init() {
 	__xgo_link_on_goexit(func() {
 		// clear when exit
 		g := uintptr(__xgo_link_getcurg())
-		mut.Lock()
+
+		// see https://github.com/xhd2015/xgo/issues/96
+		sysStack := __xgo_link_is_system_stack()
+		if !sysStack {
+			mut.Lock()
+		}
+
 		localKeys := keys
-		mut.Unlock()
+		if !sysStack {
+			mut.Unlock()
+		}
+
+		// NOTE: we must delete entries, otherwise they would
+		// cause memory leak
 		for _, loc := range localKeys {
 			loc.store.Delete(g)
 		}
 	})
+
 	__xgo_link_on_gonewproc(func(newg uintptr) {
+		// cannot lock/unlock on sys stack
+		if __xgo_link_is_system_stack() && false {
+			return
+		}
+
 		// inherit when new goroutine
 		g := uintptr(__xgo_link_getcurg())
 		mut.Lock()

diff --git a/runtime/trap/trap.go b/runtime/trap/trap.go
@@ -13,6 +13,11 @@ import (
 
 var setupOnce sync.Once
 
+func __xgo_link_is_system_stack() bool {
+	fmt.Fprintln(os.Stderr, "WARNING: failed to link __xgo_link_is_system_stack(requires xgo).")
+	return false
+}
+
 func ensureTrapInstall() {
 	setupOnce.Do(func() {
 		// set trap once needed, no matter it
@@ -37,6 +42,10 @@ func init() {
 		if isByPassing() {
 			return
 		}
+		if __xgo_link_is_system_stack() {
+			// cannot lock/unlock on sys stack
+			return
+		}
 		local := getLocalInterceptorList()
 		if !local.hasAny() {
 			return

diff --git a/script/run-test/main.go b/script/run-test/main.go
@@ -60,6 +60,7 @@ var runtimeSubTests = []string{
 	"mock_var",
 	"patch",
 	"patch_const",
+	"tls",
 }
 
 type TestCase struct {
@@ -528,6 +529,7 @@ func doRunTest(goroot string, kind testKind, args []string, tests []string) erro
 				"./trace/...",
 				"./trap/...",
 				"./mock/...",
+				"./tls/...",
 			)
 		}
 	case testKind_runtimeSubTest: