Skip to content

Commit

Permalink
Gracefully close connecting channels (#2991)
Browse files Browse the repository at this point in the history
Fixes an issue where calling dataChannel.Close on
'connecting' state channels didn't work as expected.
When handling the `OnOpen` event detect if the
user has requested close.

Fixes #2659
  • Loading branch information
joeturki authored Jan 8, 2025
1 parent 1ee0299 commit c895252
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 1 deletion.
7 changes: 6 additions & 1 deletion datachannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,13 @@ func (d *DataChannel) onMessage(msg DataChannelMessage) {

func (d *DataChannel) handleOpen(dc *datachannel.DataChannel, isRemote, isAlreadyNegotiated bool) {
d.mu.Lock()
if d.isGracefulClosed {
if d.isGracefulClosed { // The channel was closed during the connecting state
d.mu.Unlock()
if err := dc.Close(); err != nil {
d.log.Errorf("Failed to close DataChannel that was closed during connecting state %v", err.Error())
}
d.onClose()

return
}
d.dataChannel = dc
Expand Down
77 changes: 77 additions & 0 deletions datachannel_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -745,3 +745,80 @@ func TestDetachRemovesDatachannelReference(t *testing.T) {
}
}
}

func TestDataChannelClose(t *testing.T) {
// Test if onClose is fired for self and remote after Close is called
t.Run("close open channels", func(t *testing.T) {
options := &DataChannelInit{}

offerPC, answerPC, dc, done := setUpDataChannelParametersTest(t, options)

answerPC.OnDataChannel(func(dataChannel *DataChannel) {
// Make sure this is the data channel we were looking for. (Not the one
// created in signalPair).
if dataChannel.Label() != expectedLabel {
return
}

dataChannel.OnOpen(func() {
assert.NoError(t, dataChannel.Close())
})

dataChannel.OnClose(func() {
done <- true
})
})

dc.OnClose(func() {
done <- true
})

assert.NoError(t, signalPair(offerPC, answerPC))

// Offer and Answer OnClose
<-done
<-done

assert.NoError(t, offerPC.Close())
assert.NoError(t, answerPC.Close())
})

// Test if OnClose is fired for self and remote after Close is called on non-established channel
// https://github.com/pion/webrtc/issues/2659
t.Run("Close connecting channels", func(t *testing.T) {
options := &DataChannelInit{}

offerPC, answerPC, dc, done := setUpDataChannelParametersTest(t, options)

answerPC.OnDataChannel(func(dataChannel *DataChannel) {
// Make sure this is the data channel we were looking for. (Not the one
// created in signalPair).
if dataChannel.Label() != expectedLabel {
return
}

dataChannel.OnOpen(func() {
t.Fatal("OnOpen must not be fired after we call Close")
})

dataChannel.OnClose(func() {
done <- true
})

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

dc.OnClose(func() {
done <- true
})

assert.NoError(t, signalPair(offerPC, answerPC))

// Offer and Answer OnClose
<-done
<-done

assert.NoError(t, offerPC.Close())
assert.NoError(t, answerPC.Close())
})
}

0 comments on commit c895252

Please sign in to comment.