runtime: don't take allglock in tracebackothers

tracebackothers is called from fatal throw/panic.

A fatal throw may be taken with allglock held (notably in the allocator
when allglock is held), which would cause a deadlock in tracebackothers
when we try to take allglock again. Locking allglock here is also often
a lock order violation w.r.t. the locks held when throw was called.

Avoid the deadlock and ordering issues by skipping locking altogether.
It is OK to miss concurrently created Gs (which are generally avoided by
freezetheworld(), and which were possible previously anyways if created
after the loop).

Fatal throw/panic freezetheworld(), which should freeze other threads
that may be racing to modify allgs. However, freezetheworld() does _not_
guarantee that it stops all other threads, so we can't simply drop the
lock.

Fixes #42669
Updates #43175

Change-Id: I657aec46ed35fd5d1b3f1ba25b500128ab26b088
Reviewed-on: https://go-review.googlesource.com/c/go/+/270861
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Trust: Michael Pratt <mpratt@google.com>

1 files changed, 17 insertions, 10 deletions

diff --git a/src/runtime/traceback.go b/src/runtime/traceback.go
index 0825e9e7076..2601cd697f3 100644
--- a/src/runtime/traceback.go
+++ b/src/runtime/traceback.go
@@ -917,17 +917,25 @@ func tracebackothers(me *g) {
 	level, _, _ := gotraceback()
 
 	// Show the current goroutine first, if we haven't already.
-	g := getg()
-	gp := g.m.curg
-	if gp != nil && gp != me {
+	curgp := getg().m.curg
+	if curgp != nil && curgp != me {
 		print("\n")
-		goroutineheader(gp)
-		traceback(^uintptr(0), ^uintptr(0), 0, gp)
+		goroutineheader(curgp)
+		traceback(^uintptr(0), ^uintptr(0), 0, curgp)
 	}
 
-	lock(&allglock)
-	for _, gp := range allgs {
-		if gp == me || gp == g.m.curg || readgstatus(gp) == _Gdead || isSystemGoroutine(gp, false) && level < 2 {
+	// We can't take allglock here because this may be during fatal
+	// throw/panic, where locking allglock could be out-of-order or a
+	// direct deadlock.
+	//
+	// Instead, use atomic access to allgs which requires no locking. We
+	// don't lock against concurrent creation of new Gs, but even with
+	// allglock we may miss Gs created after this loop.
+	ptr, length := atomicAllG()
+	for i := uintptr(0); i < length; i++ {
+		gp := atomicAllGIndex(ptr, i)
+
+		if gp == me || gp == curgp || readgstatus(gp) == _Gdead || isSystemGoroutine(gp, false) && level < 2 {
 			continue
 		}
 		print("\n")
@@ -936,14 +944,13 @@ func tracebackothers(me *g) {
 		// called from a signal handler initiated during a
 		// systemstack call. The original G is still in the
 		// running state, and we want to print its stack.
-		if gp.m != g.m && readgstatus(gp)&^_Gscan == _Grunning {
+		if gp.m != getg().m && readgstatus(gp)&^_Gscan == _Grunning {
 			print("\tgoroutine running on other thread; stack unavailable\n")
 			printcreatedby(gp)
 		} else {
 			traceback(^uintptr(0), ^uintptr(0), 0, gp)
 		}
 	}
-	unlock(&allglock)
 }
 
 // tracebackHexdump hexdumps part of stk around frame.sp and frame.fp