Skip to content

Commit

Permalink
Avoid crash after a PC callback has been reset
Browse files Browse the repository at this point in the history
We used to crash if a PC callback was reset, due to confusion
between a nil interface and an interface whose value is nil.

Fixes #1871
  • Loading branch information
jech authored and Sean-Der committed Jul 7, 2021
1 parent d1839c7 commit ee255e8
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 8 deletions.
20 changes: 12 additions & 8 deletions peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (pc *PeerConnection) onNegotiationNeeded() {

func (pc *PeerConnection) negotiationNeededOp() {
// Don't run NegotiatedNeeded checks if OnNegotiationNeeded is not set
if handler := pc.onNegotiationNeededHandler.Load(); handler == nil {
if handler, ok := pc.onNegotiationNeededHandler.Load().(func()); !ok || handler == nil {
return
}

Expand Down Expand Up @@ -464,8 +464,8 @@ func (pc *PeerConnection) OnICEConnectionStateChange(f func(ICEConnectionState))
func (pc *PeerConnection) onICEConnectionStateChange(cs ICEConnectionState) {
pc.iceConnectionState.Store(cs)
pc.log.Infof("ICE connection state changed: %s", cs)
if handler := pc.onICEConnectionStateChangeHandler.Load(); handler != nil {
handler.(func(ICEConnectionState))(cs)
if handler, ok := pc.onICEConnectionStateChangeHandler.Load().(func(ICEConnectionState)); ok && handler != nil {
handler(cs)
}
}

Expand All @@ -475,6 +475,14 @@ func (pc *PeerConnection) OnConnectionStateChange(f func(PeerConnectionState)) {
pc.onConnectionStateChangeHandler.Store(f)
}

func (pc *PeerConnection) onConnectionStateChange(cs PeerConnectionState) {
pc.connectionState.Store(cs)
pc.log.Infof("peer connection state changed: %s", cs)
if handler, ok := pc.onConnectionStateChangeHandler.Load().(func(PeerConnectionState)); ok && handler != nil {
go handler(cs)
}
}

// SetConfiguration updates the configuration of this PeerConnection object.
func (pc *PeerConnection) SetConfiguration(configuration Configuration) error { //nolint:gocognit
// https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-setconfiguration (step #2)
Expand Down Expand Up @@ -736,11 +744,7 @@ func (pc *PeerConnection) updateConnectionState(iceConnectionState ICEConnection
return
}

pc.log.Infof("peer connection state changed: %s", connectionState)
pc.connectionState.Store(connectionState)
if handler := pc.onConnectionStateChangeHandler.Load(); handler != nil {
go handler.(func(PeerConnectionState))(connectionState)
}
pc.onConnectionStateChange(connectionState)
}

func (pc *PeerConnection) createICETransport() *ICETransport {
Expand Down
37 changes: 37 additions & 0 deletions peerconnection_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1397,3 +1397,40 @@ func TestPeerConnection_SessionID(t *testing.T) {
}
closePairNow(t, pcOffer, pcAnswer)
}

func TestPeerConnectionNilCallback(t *testing.T) {
pc, err := NewPeerConnection(Configuration{})
assert.NoError(t, err)

pc.onSignalingStateChange(SignalingStateStable)
pc.OnSignalingStateChange(func(ss SignalingState) {
t.Error("OnSignalingStateChange called")
})
pc.OnSignalingStateChange(nil)
pc.onSignalingStateChange(SignalingStateStable)

pc.onConnectionStateChange(PeerConnectionStateNew)
pc.OnConnectionStateChange(func(pcs PeerConnectionState) {
t.Error("OnConnectionStateChange called")
})
pc.OnConnectionStateChange(nil)
pc.onConnectionStateChange(PeerConnectionStateNew)

pc.onICEConnectionStateChange(ICEConnectionStateNew)
pc.OnICEConnectionStateChange(func(ics ICEConnectionState) {
t.Error("OnConnectionStateChange called")
})
pc.OnICEConnectionStateChange(nil)
pc.onICEConnectionStateChange(ICEConnectionStateNew)

pc.onNegotiationNeeded()
pc.negotiationNeededOp()
pc.OnNegotiationNeeded(func() {
t.Error("OnNegotiationNeeded called")
})
pc.OnNegotiationNeeded(nil)
pc.onNegotiationNeeded()
pc.negotiationNeededOp()

assert.NoError(t, pc.Close())
}

0 comments on commit ee255e8

Please sign in to comment.