commit ed2d5df87d0be7c86f21d19d42ec90b8d2616ae2 Author: Simone Basso bassosimone@gmail.com Date: Mon Jun 14 10:46:46 2021 +0200

Fix datarace for WebRTCPeer.lastReceive

The race condition occurs because concurrent goroutines are intermixing reads and writes of `WebRTCPeer.lastReceive`.

Spotted when integrating Snowflake inside OONI in https://github.com/ooni/probe-cli/pull/373. --- client/lib/webrtc.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/client/lib/webrtc.go b/client/lib/webrtc.go index af7ba6d..6a42ebd 100644 --- a/client/lib/webrtc.go +++ b/client/lib/webrtc.go @@ -21,8 +21,10 @@ type WebRTCPeer struct { pc *webrtc.PeerConnection transport *webrtc.DataChannel

- recvPipe *io.PipeReader - writePipe *io.PipeWriter + recvPipe *io.PipeReader + writePipe *io.PipeWriter + + mu sync.Mutex // protects the following: lastReceive time.Time

open chan struct{} // Channel to notify when datachannel opens @@ -89,12 +91,17 @@ func (c *WebRTCPeer) Close() error { // Should also update the DataChannel in underlying go-webrtc's to make Closes // more immediate / responsive. func (c *WebRTCPeer) checkForStaleness() { + c.mu.Lock() c.lastReceive = time.Now() + c.mu.Unlock() for { if c.closed { return } - if time.Since(c.lastReceive) > SnowflakeTimeout { + c.mu.Lock() + lastReceive := c.lastReceive + c.mu.Unlock() + if time.Since(lastReceive) > SnowflakeTimeout { log.Printf("WebRTC: No messages received for %v -- closing stale connection.", SnowflakeTimeout) c.Close() @@ -173,7 +180,9 @@ func (c *WebRTCPeer) preparePeerConnection(config *webrtc.Configuration) error { log.Printf("c.writePipe.CloseWithError returned error: %v", inerr) } } + c.mu.Lock() c.lastReceive = time.Now() + c.mu.Unlock() }) c.transport = dc c.open = make(chan struct{})