From 4e15fac25066d29a95b72ada046c2f0865cddce2 Mon Sep 17 00:00:00 2001 From: Joe Turki Date: Sun, 5 Jan 2025 22:41:58 -0600 Subject: [PATCH] Gracefully close connecting channels --- datachannel.go | 10 +++++- datachannel_test.go | 75 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 1 deletion(-) diff --git a/datachannel.go b/datachannel.go index 5fef7900174..053b3e0e51a 100644 --- a/datachannel.go +++ b/datachannel.go @@ -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 @@ -346,9 +349,12 @@ 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() + return } @@ -356,6 +362,8 @@ func (d *DataChannel) handleOpen(dc *datachannel.DataChannel, isRemote, isAlread d.readLoopActive = make(chan struct{}) go d.readLoop() } + + d.mu.Unlock() } // OnError sets an event handler which is invoked when diff --git a/datachannel_test.go b/datachannel_test.go index 5969da815d4..8932e9c928c 100644 --- a/datachannel_test.go +++ b/datachannel_test.go @@ -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"