fix: Update pion/webrtc to fix ICE negotiation race (#153)

* Add trace logging for pion (dtls,ice,pc)

* Temporarily disable postgres tests to spend more cycles on mock tests

* experiment: Add trace logging for WebRTC offer and answer

* Use forked pion/webrtc

Co-authored-by: Bryan Phelps <bryan@coder.com>

.github/workflows/coder.yaml

@@ -79,8 +79,7 @@ jobs:
       - uses: actions/setup-go@v2
         with:
           go-version: "^1.17"
-      - run:
-          curl -sSL
+      - run: curl -sSL
           https://github.com/kyleconroy/sqlc/releases/download/v1.11.0/sqlc_1.11.0_linux_amd64.tar.gz
           | sudo tar -C /usr/bin -xz sqlc
 
@@ -156,15 +155,13 @@ jobs:
           terraform_wrapper: false
 
       - name: Test with Mock Database
-        run:
-          gotestsum --jsonfile="gotests.json" --packages="./..." --
+        run: gotestsum --jsonfile="gotests.json" --packages="./..." --
           -covermode=atomic -coverprofile="gotests.coverage" -timeout=3m
-          -count=3 -race -short -parallel=2
+          -count=5 -race -short -parallel=2
 
       - name: Test with PostgreSQL Database
         if: runner.os == 'Linux'
-        run:
-          DB=true gotestsum --jsonfile="gotests.json" --packages="./..." --
+        run: DB=true gotestsum --jsonfile="gotests.json" --packages="./..." --
           -covermode=atomic -coverprofile="gotests.coverage" -timeout=3m
           -count=1 -race -parallel=2
 

go.mod

@@ -27,7 +27,7 @@ require (
 	github.com/pion/datachannel v1.5.2
 	github.com/pion/logging v0.2.2
 	github.com/pion/transport v0.13.0
-	github.com/pion/webrtc/v3 v3.1.19
+	github.com/pion/webrtc/v3 v3.1.21
 	github.com/spf13/cobra v1.3.0
 	github.com/stretchr/testify v1.7.0
 	github.com/unrolled/secure v1.0.9
@@ -106,7 +106,7 @@ require (
 	github.com/zclconf/go-cty v1.10.0 // indirect
 	github.com/zeebo/errs v1.2.2 // indirect
 	go.opencensus.io v0.23.0 // indirect
-	golang.org/x/net v0.0.0-20220121210141-e204ce36a2ba // indirect
+	golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect
 	golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect
 	golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
 	golang.org/x/text v0.3.7 // indirect

go.sum

@@ -1040,8 +1040,8 @@ github.com/pion/turn/v2 v2.0.6 h1:AsXjSPR6Im15DMTB39NlfdTY9BQfieANPBjdg/aVNwY=
 github.com/pion/turn/v2 v2.0.6/go.mod h1:+y7xl719J8bAEVpSXBXvTxStjJv3hbz9YFflvkpcGPw=
 github.com/pion/udp v0.1.1 h1:8UAPvyqmsxK8oOjloDk4wUt63TzFe9WEJkg5lChlj7o=
 github.com/pion/udp v0.1.1/go.mod h1:6AFo+CMdKQm7UiA0eUPA8/eVCTx8jBIITLZHc9DWX5M=
-github.com/pion/webrtc/v3 v3.1.19 h1:a4mrYskJE9LLUEogSnYVBna0Plg+fwwGLkDraz1WSwI=
-github.com/pion/webrtc/v3 v3.1.19/go.mod h1:JRlX2EANa0bbyNzbqhkSiio+tT284fgco03Xw+aS/4Q=
+github.com/pion/webrtc/v3 v3.1.21 h1:6b/65m5hSw2mF+sssHBx7Q2WPccklA0U0veEYtqZSuM=
+github.com/pion/webrtc/v3 v3.1.21/go.mod h1:dIT2ETlP5dnlkgp46fAH56UizvOKuXJ9ySgFkhtmBbw=
 github.com/pkg/browser v0.0.0-20210706143420-7d21f8c997e2/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
 github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8/go.mod h1:HKlIX3XHQyzLZPlr7++PzdhaXEj94dEiJgZDTsxEqUI=
 github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA=
@@ -1422,8 +1422,8 @@ golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qx
 golang.org/x/net v0.0.0-20211201190559-0a0e4e1bb54c/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20211216030914-fe4d6282115f/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
 golang.org/x/net v0.0.0-20220111093109-d55c255bac03/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
-golang.org/x/net v0.0.0-20220121210141-e204ce36a2ba h1:6u6sik+bn/y7vILcYkK3iwTBWN7WtBvB0+SZswQnbf8=
-golang.org/x/net v0.0.0-20220121210141-e204ce36a2ba/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
+golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd h1:O7DYs+zxREGLKzKoMQrtrEacpb0ZVXA5rIwylE2Xchk=
+golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk=
 golang.org/x/oauth2 v0.0.0-20180227000427-d7d64896b5ff/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=
 golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U=

peer/conn.go

@@ -145,39 +145,39 @@ func (c *Conn) init() error {
 
 	c.rtc.OnNegotiationNeeded(c.negotiate)
 	c.rtc.OnICEConnectionStateChange(func(iceConnectionState webrtc.ICEConnectionState) {
-		if c.isClosed() {
+		c.closedICEMutex.Lock()
+		defer c.closedICEMutex.Unlock()
+		select {
+		case <-c.closedICE:
+			// Don't log more state changes if we've already closed.
 			return
-		}
-
-		c.opts.Logger.Debug(context.Background(), "ice connection state updated",
-			slog.F("state", iceConnectionState))
+		default:
+			c.opts.Logger.Debug(context.Background(), "ice connection state updated",
+				slog.F("state", iceConnectionState))
 
-		if iceConnectionState == webrtc.ICEConnectionStateClosed {
-			// pion/webrtc can update this state multiple times.
-			// A connection can never become un-closed, so we
-			// close the channel if it isn't already.
-			c.closedICEMutex.Lock()
-			defer c.closedICEMutex.Unlock()
-			select {
-			case <-c.closedICE:
-			default:
+			if iceConnectionState == webrtc.ICEConnectionStateClosed {
+				// pion/webrtc can update this state multiple times.
+				// A connection can never become un-closed, so we
+				// close the channel if it isn't already.
 				close(c.closedICE)
 			}
 		}
 	})
 	c.rtc.OnICEGatheringStateChange(func(iceGatherState webrtc.ICEGathererState) {
-		c.opts.Logger.Debug(context.Background(), "ice gathering state updated",
-			slog.F("state", iceGatherState))
+		c.closedICEMutex.Lock()
+		defer c.closedICEMutex.Unlock()
+		select {
+		case <-c.closedICE:
+			// Don't log more state changes if we've already closed.
+			return
+		default:
+			c.opts.Logger.Debug(context.Background(), "ice gathering state updated",
+				slog.F("state", iceGatherState))
 
-		if iceGatherState == webrtc.ICEGathererStateClosed {
-			// pion/webrtc can update this state multiple times.
-			// A connection can never become un-closed, so we
-			// close the channel if it isn't already.
-			c.closedICEMutex.Lock()
-			defer c.closedICEMutex.Unlock()
-			select {
-			case <-c.closedICE:
-			default:
+			if iceGatherState == webrtc.ICEGathererStateClosed {
+				// pion/webrtc can update this state multiple times.
+				// A connection can never become un-closed, so we
+				// close the channel if it isn't already.
 				close(c.closedICE)
 			}
 		}
@@ -292,7 +292,7 @@ func (c *Conn) negotiate() {
 			_ = c.CloseWithError(xerrors.Errorf("set local description: %w", err))
 			return
 		}
-		c.opts.Logger.Debug(context.Background(), "sending offer")
+		c.opts.Logger.Debug(context.Background(), "sending offer", slog.F("offer", offer))
 		select {
 		case <-c.closed:
 			return
@@ -331,7 +331,7 @@ func (c *Conn) negotiate() {
 			_ = c.CloseWithError(xerrors.Errorf("set local description: %w", err))
 			return
 		}
-		c.opts.Logger.Debug(context.Background(), "sending answer")
+		c.opts.Logger.Debug(context.Background(), "sending answer", slog.F("answer", answer))
 		select {
 		case <-c.closed:
 			return