Skip to content

Commit

Permalink
Gracefully close connecting channels
Browse files Browse the repository at this point in the history
  • Loading branch information
JoeTurki committed Jan 6, 2025
1 parent 1ee0299 commit 4e15fac
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
10 changes: 9 additions & 1 deletion datachannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ func (d *DataChannel) handleOpen(dc *datachannel.DataChannel, isRemote, isAlread
d.mu.Lock()
if d.isGracefulClosed {
d.mu.Unlock()
dc.Close() // nolint:errcheck,gosec
d.onClose()

return
}
d.dataChannel = dc
Expand All @@ -346,16 +349,21 @@ func (d *DataChannel) handleOpen(dc *datachannel.DataChannel, isRemote, isAlread
}

d.mu.Lock()
defer d.mu.Unlock()

if d.isGracefulClosed {
d.mu.Unlock() // we unlock before we call d.onClose to avoid side effects
dc.Close() // nolint:errcheck,gosec
d.onClose()

Check warning on line 357 in datachannel.go

View check run for this annotation

Codecov / codecov/patch

datachannel.go#L354-L357

Added lines #L354 - L357 were not covered by tests
return
}

if !d.api.settingEngine.detach.DataChannels {
d.readLoopActive = make(chan struct{})
go d.readLoop()
}

d.mu.Unlock()
}

// OnError sets an event handler which is invoked when
Expand Down
75 changes: 75 additions & 0 deletions datachannel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,81 @@ func TestDataChannelParameters(t *testing.T) {
closeReliabilityParamTest(t, offerPC, answerPC, done)
})

// 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())
})

t.Run("Negotiated exchange", func(t *testing.T) {
const expectedMessage = "Hello World"

Expand Down

0 comments on commit 4e15fac

Please sign in to comment.