Skip to content

Commit

Permalink
Gracefully close connecting channels
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 and Sean-Der committed Jan 8, 2025
1 parent 1ee0299 commit 275ed7b
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 275ed7b

Please sign in to comment.