This reverts commit 7f16fcda0fd5bb625584b71311dd37b54c096136. Reason for reland: Re-landing after addressing issues in downstream code and hardening the ObserverAdapter from situations where attempted usage of data channel proxies could occur after shutting down the peer connection and terminating the network thread. Original change's description: > Revert "[DataChannel] Send and receive packets on the network thread." > > This reverts commit fe53fec24e02d2d644220f913c3f9ae596bbb2d9. > > Reason for revert: Speculative revert, may be breaking downstream project > > Original change's description: > > [DataChannel] Send and receive packets on the network thread. > > > > This updates sctp channels, including work that happens between the > > data channel controller and the transport, to run on the network > > thread. Previously all network traffic related to data channels was > > routed through the signaling thread before going to either the network > > thread or the caller's thread (e.g. js thread in chrome). Now the > > calls can go straight from the network thread to the JS thread with > > enabling a special flag on the observer (see below) and similarly > > calls to send data, involve 2 threads instead of 3. > > > > * Custom data channel observer adapter implementation that > > maintains compatibility with existing observer implementations in > > that notifications are delivered on the signaling thread. > > The adapter can be explicitly disabled for implementations that > > want to optimize the callback path and promise to not block the > > network thread. > > * Remove the signaling thread copy of data channels in the controller. > > * Remove several PostTask operations that were needed to keep things > > in sync (but the need has gone away). > > * Update tests for the controller to consistently call > > TeardownDataChannelTransport_n to match with production. > > * Update stats collectors (current and legacy) to fetch the data > > channel stats on the network thread where they're maintained. > > * Remove the AsyncChannelCloseTeardown test since the async teardown > > step has gone away. > > * Remove `sid_s` in the channel code since we only need the network > > state now. > > * For the custom observer support (with and without data adapter) and > > maintain compatibility with existing implementations, added a new > > proxy macro that allows an implementation to selectively provide > > its own implementation without being proxied. This is used for > > registering/unregistering a data channel observer. > > * Update the data channel proxy to map most methods to the network > > thread, avoiding the interim jump to the signaling thread. > > * Update a plethora of thread checkers from signaling to network. > > > > Bug: webrtc:11547 > > Change-Id: Ib4cff1482e31c46008e187189a79e967389bc518 > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/299142 > > Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> > > Reviewed-by: Henrik Boström <hbos@webrtc.org> > > Cr-Commit-Position: refs/heads/main@{#39760} > > Bug: webrtc:11547 > Change-Id: Id0d65594bf727ccea5c49093c942b09714d101ad > No-Presubmit: true > No-Tree-Checks: true > No-Try: true > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300341 > Auto-Submit: Andrey Logvin <landrey@webrtc.org> > Owners-Override: Andrey Logvin <landrey@webrtc.org> > Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com> > Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org> > Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org> > Cr-Commit-Position: refs/heads/main@{#39764} Bug: webrtc:11547 Change-Id: I47dfa7e7168be0cd2faab4f8f3ebf110c3728af5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300360 Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org> Reviewed-by: Henrik Boström <hbos@webrtc.org> Cr-Commit-Position: refs/heads/main@{#39786}
211 lines
7.8 KiB
C++
211 lines
7.8 KiB
C++
/*
|
|
* Copyright 2022 The WebRTC project authors. All Rights Reserved.
|
|
*
|
|
* Use of this source code is governed by a BSD-style license
|
|
* that can be found in the LICENSE file in the root of the source
|
|
* tree. An additional intellectual property rights grant can be found
|
|
* in the file PATENTS. All contributing project authors may
|
|
* be found in the AUTHORS file in the root of the source tree.
|
|
*/
|
|
|
|
#include "pc/data_channel_controller.h"
|
|
|
|
#include <memory>
|
|
|
|
#include "pc/peer_connection_internal.h"
|
|
#include "pc/sctp_data_channel.h"
|
|
#include "pc/test/mock_peer_connection_internal.h"
|
|
#include "rtc_base/null_socket_server.h"
|
|
#include "test/gmock.h"
|
|
#include "test/gtest.h"
|
|
#include "test/run_loop.h"
|
|
|
|
namespace webrtc {
|
|
|
|
namespace {
|
|
|
|
using ::testing::NiceMock;
|
|
using ::testing::Return;
|
|
|
|
class MockDataChannelTransport : public webrtc::DataChannelTransportInterface {
|
|
public:
|
|
~MockDataChannelTransport() override {}
|
|
|
|
MOCK_METHOD(RTCError, OpenChannel, (int channel_id), (override));
|
|
MOCK_METHOD(RTCError,
|
|
SendData,
|
|
(int channel_id,
|
|
const SendDataParams& params,
|
|
const rtc::CopyOnWriteBuffer& buffer),
|
|
(override));
|
|
MOCK_METHOD(RTCError, CloseChannel, (int channel_id), (override));
|
|
MOCK_METHOD(void, SetDataSink, (DataChannelSink * sink), (override));
|
|
MOCK_METHOD(bool, IsReadyToSend, (), (const, override));
|
|
};
|
|
|
|
// Convenience class for tests to ensure that shutdown methods for DCC
|
|
// are consistently called. In practice SdpOfferAnswerHandler will call
|
|
// TeardownDataChannelTransport_n on the network thread when destroying the
|
|
// data channel transport and PeerConnection calls PrepareForShutdown() from
|
|
// within PeerConnection::Close(). The DataChannelControllerForTest class mimics
|
|
// behavior by calling those methods from within its destructor.
|
|
class DataChannelControllerForTest : public DataChannelController {
|
|
public:
|
|
explicit DataChannelControllerForTest(PeerConnectionInternal* pc)
|
|
: DataChannelController(pc) {}
|
|
|
|
~DataChannelControllerForTest() override {
|
|
network_thread()->BlockingCall([&] { TeardownDataChannelTransport_n(); });
|
|
PrepareForShutdown();
|
|
}
|
|
};
|
|
|
|
class DataChannelControllerTest : public ::testing::Test {
|
|
protected:
|
|
DataChannelControllerTest()
|
|
: network_thread_(std::make_unique<rtc::NullSocketServer>()) {
|
|
network_thread_.Start();
|
|
pc_ = rtc::make_ref_counted<NiceMock<MockPeerConnectionInternal>>();
|
|
ON_CALL(*pc_, signaling_thread)
|
|
.WillByDefault(Return(rtc::Thread::Current()));
|
|
ON_CALL(*pc_, network_thread).WillByDefault(Return(&network_thread_));
|
|
}
|
|
|
|
~DataChannelControllerTest() override {
|
|
run_loop_.Flush();
|
|
network_thread_.Stop();
|
|
}
|
|
|
|
test::RunLoop run_loop_;
|
|
rtc::Thread network_thread_;
|
|
rtc::scoped_refptr<NiceMock<MockPeerConnectionInternal>> pc_;
|
|
};
|
|
|
|
TEST_F(DataChannelControllerTest, CreateAndDestroy) {
|
|
DataChannelControllerForTest dcc(pc_.get());
|
|
}
|
|
|
|
TEST_F(DataChannelControllerTest, CreateDataChannelEarlyRelease) {
|
|
DataChannelControllerForTest dcc(pc_.get());
|
|
auto ret = dcc.InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()));
|
|
ASSERT_TRUE(ret.ok());
|
|
auto channel = ret.MoveValue();
|
|
// DCC still holds a reference to the channel. Release this reference early.
|
|
channel = nullptr;
|
|
}
|
|
|
|
TEST_F(DataChannelControllerTest, CreateDataChannelEarlyClose) {
|
|
DataChannelControllerForTest dcc(pc_.get());
|
|
EXPECT_FALSE(dcc.HasDataChannelsForTest());
|
|
EXPECT_FALSE(dcc.HasUsedDataChannels());
|
|
auto ret = dcc.InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()));
|
|
ASSERT_TRUE(ret.ok());
|
|
auto channel = ret.MoveValue();
|
|
EXPECT_TRUE(dcc.HasDataChannelsForTest());
|
|
EXPECT_TRUE(dcc.HasUsedDataChannels());
|
|
channel->Close();
|
|
run_loop_.Flush();
|
|
EXPECT_FALSE(dcc.HasDataChannelsForTest());
|
|
EXPECT_TRUE(dcc.HasUsedDataChannels());
|
|
}
|
|
|
|
TEST_F(DataChannelControllerTest, CreateDataChannelLateRelease) {
|
|
auto dcc = std::make_unique<DataChannelControllerForTest>(pc_.get());
|
|
auto ret = dcc->InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()));
|
|
ASSERT_TRUE(ret.ok());
|
|
auto channel = ret.MoveValue();
|
|
dcc.reset();
|
|
channel = nullptr;
|
|
}
|
|
|
|
TEST_F(DataChannelControllerTest, CloseAfterControllerDestroyed) {
|
|
auto dcc = std::make_unique<DataChannelControllerForTest>(pc_.get());
|
|
auto ret = dcc->InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()));
|
|
ASSERT_TRUE(ret.ok());
|
|
auto channel = ret.MoveValue();
|
|
dcc.reset();
|
|
channel->Close();
|
|
}
|
|
|
|
// Allocate the maximum number of data channels and then one more.
|
|
// The last allocation should fail.
|
|
TEST_F(DataChannelControllerTest, MaxChannels) {
|
|
NiceMock<MockDataChannelTransport> transport;
|
|
int channel_id = 0;
|
|
|
|
ON_CALL(*pc_, GetSctpSslRole_n).WillByDefault([&]() {
|
|
return absl::optional<rtc::SSLRole>((channel_id & 1) ? rtc::SSL_SERVER
|
|
: rtc::SSL_CLIENT);
|
|
});
|
|
|
|
DataChannelControllerForTest dcc(pc_.get());
|
|
pc_->network_thread()->BlockingCall(
|
|
[&] { dcc.set_data_channel_transport(&transport); });
|
|
|
|
// Allocate the maximum number of channels + 1. Inside the loop, the creation
|
|
// process will allocate a stream id for each channel.
|
|
for (channel_id = 0; channel_id <= cricket::kMaxSctpStreams; ++channel_id) {
|
|
auto ret = dcc.InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()));
|
|
if (channel_id == cricket::kMaxSctpStreams) {
|
|
// We've reached the maximum and the previous call should have failed.
|
|
EXPECT_FALSE(ret.ok());
|
|
} else {
|
|
// We're still working on saturating the pool. Things should be working.
|
|
EXPECT_TRUE(ret.ok());
|
|
}
|
|
}
|
|
}
|
|
|
|
// Test that while a data channel is in the `kClosing` state, its StreamId does
|
|
// not get re-used for new channels. Only once the state reaches `kClosed`
|
|
// should a StreamId be available again for allocation.
|
|
TEST_F(DataChannelControllerTest, NoStreamIdReuseWhileClosing) {
|
|
ON_CALL(*pc_, GetSctpSslRole_n).WillByDefault([&]() {
|
|
return rtc::SSL_CLIENT;
|
|
});
|
|
|
|
NiceMock<MockDataChannelTransport> transport; // Wider scope than `dcc`.
|
|
DataChannelControllerForTest dcc(pc_.get());
|
|
pc_->network_thread()->BlockingCall(
|
|
[&] { dcc.set_data_channel_transport(&transport); });
|
|
|
|
// Create the first channel and check that we got the expected, first sid.
|
|
auto channel1 = dcc.InternalCreateDataChannelWithProxy(
|
|
"label", InternalDataChannelInit(DataChannelInit()))
|
|
.MoveValue();
|
|
ASSERT_EQ(channel1->id(), 0);
|
|
|
|
// Start closing the channel and make sure its state is `kClosing`
|
|
channel1->Close();
|
|
ASSERT_EQ(channel1->state(), DataChannelInterface::DataState::kClosing);
|
|
|
|
// Create a second channel and make sure we get a new StreamId, not the same
|
|
// as that of channel1.
|
|
auto channel2 = dcc.InternalCreateDataChannelWithProxy(
|
|
"label2", InternalDataChannelInit(DataChannelInit()))
|
|
.MoveValue();
|
|
ASSERT_NE(channel2->id(), channel1->id()); // In practice the id will be 2.
|
|
|
|
// Simulate the acknowledgement of the channel closing from the transport.
|
|
// This completes the closing operation of channel1.
|
|
pc_->network_thread()->BlockingCall([&] { dcc.OnChannelClosed(0); });
|
|
run_loop_.Flush();
|
|
ASSERT_EQ(channel1->state(), DataChannelInterface::DataState::kClosed);
|
|
|
|
// Now create a third channel. This time, the id of the first channel should
|
|
// be available again and therefore the ids of the first and third channels
|
|
// should be the same.
|
|
auto channel3 = dcc.InternalCreateDataChannelWithProxy(
|
|
"label3", InternalDataChannelInit(DataChannelInit()))
|
|
.MoveValue();
|
|
EXPECT_EQ(channel3->id(), channel1->id());
|
|
}
|
|
|
|
} // namespace
|
|
} // namespace webrtc
|