Skip to content

Commit 42b62fc

Browse files
committed
gopls/internal/regtest: fix data race when printing logs
Lock around log access in regtests. This flake was relatively rare in regtests because of the serial nature of our jsonrpc2 implementation, but was nonetheless racy. Fixes golang/go#41653 Change-Id: I08674f42b05624a69d33885c2232c9e31866375b Reviewed-on: https://go-review.googlesource.com/c/tools/+/257957 Trust: Rebecca Stambler <[email protected]> Trust: Robert Findley <[email protected]> Run-TryBot: Rebecca Stambler <[email protected]> Run-TryBot: Robert Findley <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Rebecca Stambler <[email protected]>
1 parent 5d1fdd8 commit 42b62fc

File tree

1 file changed

+19
-7
lines changed

1 file changed

+19
-7
lines changed

gopls/internal/regtest/runner.go

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -288,17 +288,29 @@ func (r *Runner) Run(t *testing.T, files string, test TestFunc, opts ...RunOptio
288288

289289
type loggingFramer struct {
290290
mu sync.Mutex
291-
buffers []*bytes.Buffer
291+
buffers []*safeBuffer
292+
}
293+
294+
// safeBuffer is a threadsafe buffer for logs.
295+
type safeBuffer struct {
296+
mu sync.Mutex
297+
buf bytes.Buffer
298+
}
299+
300+
func (b *safeBuffer) Write(p []byte) (int, error) {
301+
b.mu.Lock()
302+
defer b.mu.Unlock()
303+
return b.buf.Write(p)
292304
}
293305

294306
func (s *loggingFramer) framer(f jsonrpc2.Framer) jsonrpc2.Framer {
295307
return func(nc net.Conn) jsonrpc2.Stream {
296308
s.mu.Lock()
297-
var buf bytes.Buffer
298-
s.buffers = append(s.buffers, &buf)
309+
buf := &safeBuffer{buf: bytes.Buffer{}}
310+
s.buffers = append(s.buffers, buf)
299311
s.mu.Unlock()
300312
stream := f(nc)
301-
return protocol.LoggingStream(stream, &buf)
313+
return protocol.LoggingStream(stream, buf)
302314
}
303315
}
304316

@@ -308,9 +320,9 @@ func (s *loggingFramer) printBuffers(testname string, w io.Writer) {
308320

309321
for i, buf := range s.buffers {
310322
fmt.Fprintf(os.Stderr, "#### Start Gopls Test Logs %d of %d for %q\n", i+1, len(s.buffers), testname)
311-
// Re-buffer buf to avoid a data rate (io.Copy mutates src).
312-
writeBuf := bytes.NewBuffer(buf.Bytes())
313-
io.Copy(w, writeBuf)
323+
buf.mu.Lock()
324+
io.Copy(w, &buf.buf)
325+
buf.mu.Unlock()
314326
fmt.Fprintf(os.Stderr, "#### End Gopls Test Logs %d of %d for %q\n", i+1, len(s.buffers), testname)
315327
}
316328
}

0 commit comments

Comments
 (0)