Skip to content

Commit 2a98a18

Browse files
committed
runtime: uphold goroutine profile invariants in coroswitch
Goroutine profiles require checking in with the profiler before any goroutine starts running. coroswitch is a place where a goroutine may start running, but where we do not check in with the profiler, which leads to crashes. Fix this by checking in with the profiler the same way execute does. Fixes #69998. Change-Id: Idef6dd31b70a73dd1c967b56c307c7a46a26ba73 Reviewed-on: https://go-review.googlesource.com/c/go/+/622016 Reviewed-by: David Chase <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent fb2a7f8 commit 2a98a18

File tree

2 files changed

+57
-0
lines changed

2 files changed

+57
-0
lines changed

src/runtime/coro.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,6 +211,18 @@ func coroswitch_m(gp *g) {
211211
// directly if possible.
212212
setGNoWB(&mp.curg, gnext)
213213
setMNoWB(&gnext.m, mp)
214+
215+
// Synchronize with any out-standing goroutine profile. We're about to start
216+
// executing, and an invariant of the profiler is that we tryRecordGoroutineProfile
217+
// whenever a goroutine is about to start running.
218+
//
219+
// N.B. We must do this before transitioning to _Grunning but after installing gnext
220+
// in curg, so that we have a valid curg for allocation (tryRecordGoroutineProfile
221+
// may allocate).
222+
if goroutineProfile.active {
223+
tryRecordGoroutineProfile(gnext, nil, osyield)
224+
}
225+
214226
if !gnext.atomicstatus.CompareAndSwap(_Gwaiting, _Grunning) {
215227
// The CAS failed: use casgstatus, which will take care of
216228
// coordinating with the garbage collector about the state change.

src/runtime/pprof/pprof_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"internal/syscall/unix"
1616
"internal/testenv"
1717
"io"
18+
"iter"
1819
"math"
1920
"math/big"
2021
"os"
@@ -1784,6 +1785,50 @@ func TestGoroutineProfileConcurrency(t *testing.T) {
17841785
}
17851786
}
17861787

1788+
// Regression test for #69998.
1789+
func TestGoroutineProfileCoro(t *testing.T) {
1790+
testenv.MustHaveParallelism(t)
1791+
1792+
goroutineProf := Lookup("goroutine")
1793+
1794+
// Set up a goroutine to just create and run coroutine goroutines all day.
1795+
iterFunc := func() {
1796+
p, stop := iter.Pull2(
1797+
func(yield func(int, int) bool) {
1798+
for i := 0; i < 10000; i++ {
1799+
if !yield(i, i) {
1800+
return
1801+
}
1802+
}
1803+
},
1804+
)
1805+
defer stop()
1806+
for {
1807+
_, _, ok := p()
1808+
if !ok {
1809+
break
1810+
}
1811+
}
1812+
}
1813+
var wg sync.WaitGroup
1814+
done := make(chan struct{})
1815+
wg.Add(1)
1816+
go func() {
1817+
defer wg.Done()
1818+
for {
1819+
iterFunc()
1820+
select {
1821+
case <-done:
1822+
default:
1823+
}
1824+
}
1825+
}()
1826+
1827+
// Take a goroutine profile. If the bug in #69998 is present, this will crash
1828+
// with high probability. We don't care about the output for this bug.
1829+
goroutineProf.WriteTo(io.Discard, 1)
1830+
}
1831+
17871832
func BenchmarkGoroutine(b *testing.B) {
17881833
withIdle := func(n int, fn func(b *testing.B)) func(b *testing.B) {
17891834
return func(b *testing.B) {

0 commit comments

Comments
 (0)