Fix: CreateOffer race condition #1414
Conversation
| } | ||
|
|
||
| // Wait for async to complete otherwise dangling renegotation | ||
| // will occur on closed connection and throw InvalidStateError: connection closed |
There was a problem hiding this comment.
This is #1401. I will fix it in a separate PR.
Codecov Report
@@ Coverage Diff @@
## master #1414 +/- ##
==========================================
+ Coverage 75.75% 76.04% +0.29%
==========================================
Files 73 73
Lines 5948 5962 +14
==========================================
+ Hits 4506 4534 +28
+ Misses 1095 1078 -17
- Partials 347 350 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
|
||
| // in-parallel steps to create an offer | ||
| // https://w3c.github.io/webrtc-pc/#dfn-in-parallel-steps-to-create-an-offer | ||
| isPlanB := pc.configuration.SDPSemantics == SDPSemanticsPlanB |
There was a problem hiding this comment.
the only modification between here and line 653 is to use currentTransceivers rather than pc.GetTransceivers()
tarrencev
force-pushed
the
fix/races2
branch
3 times, most recently
from
1275692 to
d3a79a9
Compare
Fixes a race condition that occurs when the local media (transceivers) are mutated during the offer generation process.
tarrencev
force-pushed
the
fix/races2
branch
from
d3a79a9 to
6a1b906
Compare
| @@ -348,7 +348,7 @@ func (pc *PeerConnection) checkNegotiationNeeded() bool { | |||
| for _, t := range pc.GetTransceivers() { | |||
| // https://www.w3.org/TR/webrtc/#dfn-update-the-negotiation-needed-flag | |||
| // Step 5.1 | |||
| // if t.stoping && !t.stopped { | |||
There was a problem hiding this comment.
Should these be commented out? They were in the original PR, maybe we should just delete?
There was a problem hiding this comment.
according to the spec we should have a stopping property on a transceiver but we dont right now. so this was added prematurely. i'll leave for now and we can remove it separately
Fixes a race condition that occurs when the local media (transceivers) are mutated during the offer generation process.
Resolves an issue where an offer is created with no
midcausing the error below:The test is not great, since it depends on the performance of the executor. I was able to consistently reproduce locally with it though.