Move lock to handshake so no race can occur

kylecarbs · kylecarbs · commit 7321303879d5 · 2022-01-27T14:40:10.000Z
diff --git a/peer/conn.go b/peer/conn.go
@@ -280,8 +280,11 @@ func (c *Conn) negotiate() {
 	case remoteDescription = <-c.remoteSessionDescriptionChannel:
 	}
 
+	// Must lock new candidates from being sent while the description is being flushed.
+	c.pendingCandidatesMutex.Lock()
 	err := c.rtc.SetRemoteDescription(remoteDescription)
 	if err != nil {
+		c.pendingCandidatesMutex.Unlock()
 		_ = c.CloseWithError(xerrors.Errorf("set remote description (closed %v): %w", c.isClosed(), err))
 		return
 	}
@@ -291,8 +294,11 @@ func (c *Conn) negotiate() {
 		// time. If candidates flush before this point, a connection could fail.
 		c.flushPendingCandidates()
 	}
+	c.pendingCandidatesMutex.Unlock()
 
 	if !c.offerrer {
+		c.pendingCandidatesMutex.Lock()
+
 		answer, err := c.rtc.CreateAnswer(&webrtc.AnswerOptions{})
 		if err != nil {
 			_ = c.CloseWithError(xerrors.Errorf("create answer: %w", err))
@@ -314,13 +320,13 @@ func (c *Conn) negotiate() {
 
 		// Wait until the local description is set to flush candidates.
 		c.flushPendingCandidates()
+		c.pendingCandidatesMutex.Unlock()
 	}
 }
 
 // flushPendingCandidates writes all local candidates to the candidate send channel.
 // The localCandidateChannel is expected to be serviced, otherwise this could block.
 func (c *Conn) flushPendingCandidates() {
-	c.pendingCandidatesMutex.Lock()
 	for _, localCandidate := range c.pendingCandidates {
 		c.opts.Logger.Debug(context.Background(), "flushing local candidate")
 		select {
@@ -331,7 +337,6 @@ func (c *Conn) flushPendingCandidates() {
 	}
 	c.pendingCandidates = make([]webrtc.ICECandidateInit, 0)
 	c.opts.Logger.Debug(context.Background(), "flushed candidates")
-	c.pendingCandidatesMutex.Unlock()
 }
 
 // LocalCandidate returns a channel that emits when a local candidate

Original file line number	Diff line number	Diff line change
`@@ -280,8 +280,11 @@ func (c *Conn) negotiate() {`
`280`	`280`	`case remoteDescription = <-c.remoteSessionDescriptionChannel:`
`281`	`281`	`}`
`282`	`282`
	`283`	`+ // Must lock new candidates from being sent while the description is being flushed.`
	`284`	`+ c.pendingCandidatesMutex.Lock()`
`283`	`285`	`err := c.rtc.SetRemoteDescription(remoteDescription)`
`284`	`286`	`if err != nil {`
	`287`	`+ c.pendingCandidatesMutex.Unlock()`
`285`	`288`	`_ = c.CloseWithError(xerrors.Errorf("set remote description (closed %v): %w", c.isClosed(), err))`
`286`	`289`	`return`
`287`	`290`	`}`
`@@ -291,8 +294,11 @@ func (c *Conn) negotiate() {`
`291`	`294`	`// time. If candidates flush before this point, a connection could fail.`
`292`	`295`	`c.flushPendingCandidates()`
`293`	`296`	`}`
	`297`	`+ c.pendingCandidatesMutex.Unlock()`
`294`	`298`
`295`	`299`	`if !c.offerrer {`
	`300`	`+ c.pendingCandidatesMutex.Lock()`
	`301`	`+`
`296`	`302`	`answer, err := c.rtc.CreateAnswer(&webrtc.AnswerOptions{})`
`297`	`303`	`if err != nil {`
`298`	`304`	`_ = c.CloseWithError(xerrors.Errorf("create answer: %w", err))`
`@@ -314,13 +320,13 @@ func (c *Conn) negotiate() {`
`314`	`320`
`315`	`321`	`// Wait until the local description is set to flush candidates.`
`316`	`322`	`c.flushPendingCandidates()`
	`323`	`+ c.pendingCandidatesMutex.Unlock()`
`317`	`324`	`}`
`318`	`325`	`}`
`319`	`326`
`320`	`327`	`// flushPendingCandidates writes all local candidates to the candidate send channel.`
`321`	`328`	`// The localCandidateChannel is expected to be serviced, otherwise this could block.`
`322`	`329`	`func (c *Conn) flushPendingCandidates() {`
`323`		`- c.pendingCandidatesMutex.Lock()`
`324`	`330`	`for _, localCandidate := range c.pendingCandidates {`
`325`	`331`	`c.opts.Logger.Debug(context.Background(), "flushing local candidate")`
`326`	`332`	`select {`
`@@ -331,7 +337,6 @@ func (c *Conn) flushPendingCandidates() {`
`331`	`337`	`}`
`332`	`338`	`c.pendingCandidates = make([]webrtc.ICECandidateInit, 0)`
`333`	`339`	`c.opts.Logger.Debug(context.Background(), "flushed candidates")`
`334`		`- c.pendingCandidatesMutex.Unlock()`
`335`	`340`	`}`
`336`	`341`
`337`	`342`	`// LocalCandidate returns a channel that emits when a local candidate`