Skip to content

fix: Lock when obtaining a peer connection answer<->offer #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jan 13, 2022
Merged

fix: Lock when obtaining a peer connection answer<->offer #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
117bae0
fix: Lock when obtaining a peer connection answer<->offer
kylecarbs Jan 13, 2022
a99e8e6
Remove unnecessary log
kylecarbs Jan 13, 2022
7923caa
Merge branch 'main' into peerflakey
kylecarbs Jan 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions peer/conn.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ type Conn struct {
localCandidateChannel chan webrtc.ICECandidateInit
localSessionDescriptionChannel chan webrtc.SessionDescription
remoteSessionDescriptionChannel chan webrtc.SessionDescription
remoteSessionDescriptionMutex sync.Mutex

pingChannelID uint16
pingEchoChannelID uint16
Expand Down Expand Up @@ -228,6 +229,11 @@ func (c *Conn) negotiate() {
c.opts.Logger.Debug(context.Background(), "negotiating")
flushCandidates := c.proxyICECandidates()

// Locks while the negotiation for a remote session
// description is taking place.
c.remoteSessionDescriptionMutex.Lock()
defer c.remoteSessionDescriptionMutex.Unlock()

if c.offerrer {
offer, err := c.rtc.CreateOffer(&webrtc.OfferOptions{})
if err != nil {
Expand Down Expand Up @@ -328,6 +334,9 @@ func (c *Conn) LocalCandidate() <-chan webrtc.ICECandidateInit {

// AddRemoteCandidate adds a remote candidate to the RTC connection.
func (c *Conn) AddRemoteCandidate(i webrtc.ICECandidateInit) error {
// Prevents candidates from being added before an offer<->answer has occurred.
c.remoteSessionDescriptionMutex.Lock()
defer c.remoteSessionDescriptionMutex.Unlock()
return c.rtc.AddICECandidate(i)
}

Expand Down
2 changes: 1 addition & 1 deletion peerbroker/dial.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func Dial(stream proto.DRPCPeerBroker_NegotiateConnectionClient, iceServers []we
for {
serverToClientMessage, err := stream.Recv()
if err != nil {
_ = peerConn.CloseWithError(err)
_ = peerConn.CloseWithError(xerrors.Errorf("recv: %w", err))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Glad to have more info when this comes up!

return
}

Expand Down
3 changes: 3 additions & 0 deletions peerbroker/dial_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ func TestMain(m *testing.M) {
}

func TestDial(t *testing.T) {
t.Parallel()

t.Run("Connect", func(t *testing.T) {
t.Parallel()
ctx := context.Background()
client, server := provisionersdk.TransportPipe()
defer client.Close()
Expand Down