Skip to content

Commit

Permalink
TBufferedServer: Avoid channel close/send race on Stop (#2583)
Browse files Browse the repository at this point in the history
TBufferedServer.Close() may close the data channel before
Serve detects it. Move chan close() to Serve to prevent
a panic caused by sending on a closed channel.

See also golang/go#27769 (comment)

	=== RUN   TestTBufferedServer_SendReceive
	==================
	WARNING: DATA RACE
	Write at 0x00c000074850 by goroutine 9:
	  runtime.closechan()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:352 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Stop()
	...
	Previous read at 0x00c000074850 by goroutine 10:
	  runtime.chansend()
	      /home/travis/.gimme/versions/go1.15.2.linux.amd64/src/runtime/chan.go:158 +0x0
	  github.com/jaegertracing/jaeger/cmd/agent/app/servers.(*TBufferedServer).Serve()
	      /home/travis/gopath/src/github.com/jaegertracing/jaeger/cmd/agent/app/servers/tbuffered_server.go:97 +0x264
	...
	==================
	    testing.go:1042: race detected during execution of test
	--- FAIL: TestTBufferedServer_SendReceive (0.01s)

Closes #2577

Signed-off-by: Carl Henrik Lunde <chlunde@ifi.uio.no>
  • Loading branch information
chlunde authored Oct 25, 2020
1 parent 20d5459 commit 2bce5ad
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion cmd/agent/app/servers/tbuffered_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ func (s *TBufferedServer) Serve() {
s.metrics.ReadError.Inc(1)
}
}
close(s.dataChan)
}

func (s *TBufferedServer) updateQueueSize(delta int64) {
Expand All @@ -121,7 +122,6 @@ func (s *TBufferedServer) IsServing() bool {
func (s *TBufferedServer) Stop() {
atomic.StoreUint32(&s.serving, 0)
_ = s.transport.Close()
close(s.dataChan)
}

// DataChan returns the data chan of the buffered server
Expand Down

0 comments on commit 2bce5ad

Please sign in to comment.