Skip to content

Commit aad0180

Browse files
neildgopherbot
authored andcommitted
http2: fix flakiness from t.Log when GOOS=js
The http2 package uses a precursor to the experimental testing/synctest package, parsing runtime.Stack output to determine when goroutines are idle. When GOOS=js, some tests which use t.Log are flaky. t.Log blocks in the syscall package writing to stdout. The GOOS=js implementation of the syscall leaves the goroutine blocked on a channel operation, which synctest interprets as the goroutine being "durably blocked". Fix the http2 synctest to treat any goroutine blocked in the syscall package as not being durably blocked. Making this fix reveals another bug when GOOS=js: Looping while calling runtime.Gosched does not appear to permit syscalls to make progress. Add a few time.Sleep(1) calls while waiting for idleness to work around the problem. While changing things in here, change http2's synctest to not treat goroutines blocked on mutex operations as durably blocked. This matches the behavior of testing/synctest. (This would all be simpler if we just used testing/synctest, but we don't want to make the http2 package depend on an experimental API.) Fixes golang/go#67693 Change-Id: I889834e97e4a33f4ef232278b1a78af00d52d261 Reviewed-on: https://go-review.googlesource.com/c/net/+/653696 Reviewed-by: Jonathan Amsterdam <jba@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Damien Neil <dneil@google.com>
1 parent b73e574 commit aad0180

File tree

1 file changed

+45
-9
lines changed

1 file changed

+45
-9
lines changed

http2/sync_test.go

+45-9
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,10 @@ type synctestGroup struct {
2424
}
2525

2626
type goroutine struct {
27-
id int
28-
parent int
29-
state string
27+
id int
28+
parent int
29+
state string
30+
syscall bool
3031
}
3132

3233
// newSynctest creates a new group with the synthetic clock set the provided time.
@@ -76,6 +77,14 @@ func (g *synctestGroup) Wait() {
7677
return
7778
}
7879
runtime.Gosched()
80+
if runtime.GOOS == "js" {
81+
// When GOOS=js, we appear to need to time.Sleep to make progress
82+
// on some syscalls. In particular, without this sleep
83+
// writing to stdout (including via t.Log) can block forever.
84+
for range 10 {
85+
time.Sleep(1)
86+
}
87+
}
7988
}
8089
}
8190

@@ -87,6 +96,9 @@ func (g *synctestGroup) idle() bool {
8796
if !g.gids[gr.id] && !g.gids[gr.parent] {
8897
continue
8998
}
99+
if gr.syscall {
100+
return false
101+
}
90102
// From runtime/runtime2.go.
91103
switch gr.state {
92104
case "IO wait":
@@ -97,9 +109,6 @@ func (g *synctestGroup) idle() bool {
97109
case "chan receive":
98110
case "chan send":
99111
case "sync.Cond.Wait":
100-
case "sync.Mutex.Lock":
101-
case "sync.RWMutex.RLock":
102-
case "sync.RWMutex.Lock":
103112
default:
104113
return false
105114
}
@@ -138,6 +147,10 @@ func stacks(all bool) []goroutine {
138147
panic(fmt.Errorf("3 unparsable goroutine stack:\n%s", gs))
139148
}
140149
state, rest, ok := strings.Cut(rest, "]")
150+
isSyscall := false
151+
if strings.Contains(rest, "\nsyscall.") {
152+
isSyscall = true
153+
}
141154
var parent int
142155
_, rest, ok = strings.Cut(rest, "\ncreated by ")
143156
if ok && strings.Contains(rest, " in goroutine ") {
@@ -155,9 +168,10 @@ func stacks(all bool) []goroutine {
155168
}
156169
}
157170
goroutines = append(goroutines, goroutine{
158-
id: id,
159-
parent: parent,
160-
state: state,
171+
id: id,
172+
parent: parent,
173+
state: state,
174+
syscall: isSyscall,
161175
})
162176
}
163177
return goroutines
@@ -291,3 +305,25 @@ func (tm *fakeTimer) Stop() bool {
291305
delete(tm.g.timers, tm)
292306
return stopped
293307
}
308+
309+
// TestSynctestLogs verifies that t.Log works,
310+
// in particular that the GOOS=js workaround in synctestGroup.Wait is working.
311+
// (When GOOS=js, writing to stdout can hang indefinitely if some goroutine loops
312+
// calling runtime.Gosched; see Wait for the workaround.)
313+
func TestSynctestLogs(t *testing.T) {
314+
g := newSynctest(time.Now())
315+
donec := make(chan struct{})
316+
go func() {
317+
g.Join()
318+
for range 100 {
319+
t.Logf("logging a long line")
320+
}
321+
close(donec)
322+
}()
323+
g.Wait()
324+
select {
325+
case <-donec:
326+
default:
327+
panic("done")
328+
}
329+
}

0 commit comments

Comments
 (0)