Add thread annotations to FakeIceTransport
Bug: webrtc:12339
Change-Id: I29f5c910c60155cbb48c686e77b02ad3aa761fb1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/211665
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33500}
diff --git a/p2p/base/fake_dtls_transport.h b/p2p/base/fake_dtls_transport.h
index f17eddf..daec158 100644
--- a/p2p/base/fake_dtls_transport.h
+++ b/p2p/base/fake_dtls_transport.h
@@ -55,9 +55,15 @@
// If this constructor is called, a new fake ICE transport will be created,
// and this FakeDtlsTransport will take the ownership.
- explicit FakeDtlsTransport(const std::string& name, int component)
+ FakeDtlsTransport(const std::string& name, int component)
: FakeDtlsTransport(std::make_unique<FakeIceTransport>(name, component)) {
}
+ FakeDtlsTransport(const std::string& name,
+ int component,
+ rtc::Thread* network_thread)
+ : FakeDtlsTransport(std::make_unique<FakeIceTransport>(name,
+ component,
+ network_thread)) {}
~FakeDtlsTransport() override {
if (dest_ && dest_->dest_ == this) {
diff --git a/p2p/base/fake_ice_transport.h b/p2p/base/fake_ice_transport.h
index f39da7c..58d83d7 100644
--- a/p2p/base/fake_ice_transport.h
+++ b/p2p/base/fake_ice_transport.h
@@ -25,6 +25,9 @@
namespace cricket {
+// All methods must be called on the network thread (which is either the thread
+// calling the constructor, or the separate thread explicitly passed to the
+// constructor).
class FakeIceTransport : public IceTransportInternal {
public:
explicit FakeIceTransport(const std::string& name,
@@ -34,6 +37,8 @@
component_(component),
network_thread_(network_thread ? network_thread
: rtc::Thread::Current()) {}
+ // Must be called either on the network thread, or after the network thread
+ // has been shut down.
~FakeIceTransport() override {
if (dest_ && dest_->dest_ == this) {
dest_->dest_ = nullptr;
@@ -42,18 +47,31 @@
// If async, will send packets by "Post"-ing to message queue instead of
// synchronously "Send"-ing.
- void SetAsync(bool async) { async_ = async; }
- void SetAsyncDelay(int delay_ms) { async_delay_ms_ = delay_ms; }
+ void SetAsync(bool async) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ async_ = async;
+ }
+ void SetAsyncDelay(int delay_ms) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ async_delay_ms_ = delay_ms;
+ }
// SetWritable, SetReceiving and SetDestination are the main methods that can
// be used for testing, to simulate connectivity or lack thereof.
- void SetWritable(bool writable) { set_writable(writable); }
- void SetReceiving(bool receiving) { set_receiving(receiving); }
+ void SetWritable(bool writable) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ set_writable(writable);
+ }
+ void SetReceiving(bool receiving) {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ set_receiving(receiving);
+ }
// Simulates the two transports connecting to each other.
// If |asymmetric| is true this method only affects this FakeIceTransport.
// If false, it affects |dest| as well.
void SetDestination(FakeIceTransport* dest, bool asymmetric = false) {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (dest == dest_) {
return;
}
@@ -75,12 +93,14 @@
void SetTransportState(webrtc::IceTransportState state,
IceTransportState legacy_state) {
+ RTC_DCHECK_RUN_ON(network_thread_);
transport_state_ = state;
legacy_transport_state_ = legacy_state;
SignalIceTransportStateChanged(this);
}
void SetConnectionCount(size_t connection_count) {
+ RTC_DCHECK_RUN_ON(network_thread_);
size_t old_connection_count = connection_count_;
connection_count_ = connection_count;
if (connection_count) {
@@ -94,6 +114,7 @@
}
void SetCandidatesGatheringComplete() {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (gathering_state_ != kIceGatheringComplete) {
gathering_state_ = kIceGatheringComplete;
SignalGatheringState(this);
@@ -102,16 +123,29 @@
// Convenience functions for accessing ICE config and other things.
int receiving_timeout() const {
+ RTC_DCHECK_RUN_ON(network_thread_);
return ice_config_.receiving_timeout_or_default();
}
- bool gather_continually() const { return ice_config_.gather_continually(); }
- const Candidates& remote_candidates() const { return remote_candidates_; }
+ bool gather_continually() const {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return ice_config_.gather_continually();
+ }
+ const Candidates& remote_candidates() const {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return remote_candidates_;
+ }
// Fake IceTransportInternal implementation.
const std::string& transport_name() const override { return name_; }
int component() const override { return component_; }
- uint64_t IceTiebreaker() const { return tiebreaker_; }
- IceMode remote_ice_mode() const { return remote_ice_mode_; }
+ uint64_t IceTiebreaker() const {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return tiebreaker_;
+ }
+ IceMode remote_ice_mode() const {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return remote_ice_mode_;
+ }
const std::string& ice_ufrag() const { return ice_parameters_.ufrag; }
const std::string& ice_pwd() const { return ice_parameters_.pwd; }
const std::string& remote_ice_ufrag() const {
@@ -126,6 +160,7 @@
}
IceTransportState GetState() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (legacy_transport_state_) {
return *legacy_transport_state_;
}
@@ -143,6 +178,7 @@
}
webrtc::IceTransportState GetIceTransportState() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (transport_state_) {
return *transport_state_;
}
@@ -159,21 +195,34 @@
return webrtc::IceTransportState::kConnected;
}
- void SetIceRole(IceRole role) override { role_ = role; }
- IceRole GetIceRole() const override { return role_; }
+ void SetIceRole(IceRole role) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ role_ = role;
+ }
+ IceRole GetIceRole() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return role_;
+ }
void SetIceTiebreaker(uint64_t tiebreaker) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
tiebreaker_ = tiebreaker;
}
void SetIceParameters(const IceParameters& ice_params) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
ice_parameters_ = ice_params;
}
void SetRemoteIceParameters(const IceParameters& params) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
remote_ice_parameters_ = params;
}
- void SetRemoteIceMode(IceMode mode) override { remote_ice_mode_ = mode; }
+ void SetRemoteIceMode(IceMode mode) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ remote_ice_mode_ = mode;
+ }
void MaybeStartGathering() override {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (gathering_state_ == kIceGatheringNew) {
gathering_state_ = kIceGatheringGathering;
SignalGatheringState(this);
@@ -181,15 +230,21 @@
}
IceGatheringState gathering_state() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
return gathering_state_;
}
- void SetIceConfig(const IceConfig& config) override { ice_config_ = config; }
+ void SetIceConfig(const IceConfig& config) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ ice_config_ = config;
+ }
void AddRemoteCandidate(const Candidate& candidate) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
remote_candidates_.push_back(candidate);
}
void RemoveRemoteCandidate(const Candidate& candidate) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
auto it = absl::c_find(remote_candidates_, candidate);
if (it == remote_candidates_.end()) {
RTC_LOG(LS_INFO) << "Trying to remove a candidate which doesn't exist.";
@@ -199,7 +254,10 @@
remote_candidates_.erase(it);
}
- void RemoveAllRemoteCandidates() override { remote_candidates_.clear(); }
+ void RemoveAllRemoteCandidates() override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ remote_candidates_.clear();
+ }
bool GetStats(IceTransportStats* ice_transport_stats) override {
CandidateStats candidate_stats;
@@ -220,17 +278,25 @@
}
// Fake PacketTransportInternal implementation.
- bool writable() const override { return writable_; }
- bool receiving() const override { return receiving_; }
+ bool writable() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return writable_;
+ }
+ bool receiving() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return receiving_;
+ }
// If combine is enabled, every two consecutive packets to be sent with
// "SendPacket" will be combined into one outgoing packet.
void combine_outgoing_packets(bool combine) {
+ RTC_DCHECK_RUN_ON(network_thread_);
combine_outgoing_packets_ = combine;
}
int SendPacket(const char* data,
size_t len,
const rtc::PacketOptions& options,
int flags) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
if (!dest_) {
return -1;
}
@@ -240,8 +306,11 @@
rtc::CopyOnWriteBuffer packet(std::move(send_packet_));
if (async_) {
invoker_.AsyncInvokeDelayed<void>(
- RTC_FROM_HERE, rtc::Thread::Current(),
- [this, packet] { FakeIceTransport::SendPacketInternal(packet); },
+ RTC_FROM_HERE, network_thread_,
+ [this, packet] {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ FakeIceTransport::SendPacketInternal(packet);
+ },
async_delay_ms_);
} else {
SendPacketInternal(packet);
@@ -253,10 +322,12 @@
}
int SetOption(rtc::Socket::Option opt, int value) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
socket_options_[opt] = value;
return true;
}
bool GetOption(rtc::Socket::Option opt, int* value) override {
+ RTC_DCHECK_RUN_ON(network_thread_);
auto it = socket_options_.find(opt);
if (it != socket_options_.end()) {
*value = it->second;
@@ -268,19 +339,27 @@
int GetError() override { return 0; }
- rtc::CopyOnWriteBuffer last_sent_packet() { return last_sent_packet_; }
+ rtc::CopyOnWriteBuffer last_sent_packet() {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ return last_sent_packet_;
+ }
absl::optional<rtc::NetworkRoute> network_route() const override {
+ RTC_DCHECK_RUN_ON(network_thread_);
return network_route_;
}
void SetNetworkRoute(absl::optional<rtc::NetworkRoute> network_route) {
+ RTC_DCHECK_RUN_ON(network_thread_);
network_route_ = network_route;
- network_thread_->Invoke<void>(
- RTC_FROM_HERE, [this] { SignalNetworkRouteChanged(network_route_); });
+ network_thread_->Invoke<void>(RTC_FROM_HERE, [this] {
+ RTC_DCHECK_RUN_ON(network_thread_);
+ SignalNetworkRouteChanged(network_route_);
+ });
}
private:
- void set_writable(bool writable) {
+ void set_writable(bool writable)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(network_thread_) {
if (writable_ == writable) {
return;
}
@@ -292,7 +371,8 @@
SignalWritableState(this);
}
- void set_receiving(bool receiving) {
+ void set_receiving(bool receiving)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(network_thread_) {
if (receiving_ == receiving) {
return;
}
@@ -300,7 +380,8 @@
SignalReceivingState(this);
}
- void SendPacketInternal(const rtc::CopyOnWriteBuffer& packet) {
+ void SendPacketInternal(const rtc::CopyOnWriteBuffer& packet)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(network_thread_) {
if (dest_) {
last_sent_packet_ = packet;
dest_->SignalReadPacket(dest_, packet.data<char>(), packet.size(),
@@ -309,30 +390,35 @@
}
rtc::AsyncInvoker invoker_;
- std::string name_;
- int component_;
- FakeIceTransport* dest_ = nullptr;
- bool async_ = false;
- int async_delay_ms_ = 0;
- Candidates remote_candidates_;
- IceConfig ice_config_;
- IceRole role_ = ICEROLE_UNKNOWN;
- uint64_t tiebreaker_ = 0;
- IceParameters ice_parameters_;
- IceParameters remote_ice_parameters_;
- IceMode remote_ice_mode_ = ICEMODE_FULL;
- size_t connection_count_ = 0;
- absl::optional<webrtc::IceTransportState> transport_state_;
- absl::optional<IceTransportState> legacy_transport_state_;
- IceGatheringState gathering_state_ = kIceGatheringNew;
- bool had_connection_ = false;
- bool writable_ = false;
- bool receiving_ = false;
- bool combine_outgoing_packets_ = false;
- rtc::CopyOnWriteBuffer send_packet_;
- absl::optional<rtc::NetworkRoute> network_route_;
- std::map<rtc::Socket::Option, int> socket_options_;
- rtc::CopyOnWriteBuffer last_sent_packet_;
+ const std::string name_;
+ const int component_;
+ FakeIceTransport* dest_ RTC_GUARDED_BY(network_thread_) = nullptr;
+ bool async_ RTC_GUARDED_BY(network_thread_) = false;
+ int async_delay_ms_ RTC_GUARDED_BY(network_thread_) = 0;
+ Candidates remote_candidates_ RTC_GUARDED_BY(network_thread_);
+ IceConfig ice_config_ RTC_GUARDED_BY(network_thread_);
+ IceRole role_ RTC_GUARDED_BY(network_thread_) = ICEROLE_UNKNOWN;
+ uint64_t tiebreaker_ RTC_GUARDED_BY(network_thread_) = 0;
+ IceParameters ice_parameters_ RTC_GUARDED_BY(network_thread_);
+ IceParameters remote_ice_parameters_ RTC_GUARDED_BY(network_thread_);
+ IceMode remote_ice_mode_ RTC_GUARDED_BY(network_thread_) = ICEMODE_FULL;
+ size_t connection_count_ RTC_GUARDED_BY(network_thread_) = 0;
+ absl::optional<webrtc::IceTransportState> transport_state_
+ RTC_GUARDED_BY(network_thread_);
+ absl::optional<IceTransportState> legacy_transport_state_
+ RTC_GUARDED_BY(network_thread_);
+ IceGatheringState gathering_state_ RTC_GUARDED_BY(network_thread_) =
+ kIceGatheringNew;
+ bool had_connection_ RTC_GUARDED_BY(network_thread_) = false;
+ bool writable_ RTC_GUARDED_BY(network_thread_) = false;
+ bool receiving_ RTC_GUARDED_BY(network_thread_) = false;
+ bool combine_outgoing_packets_ RTC_GUARDED_BY(network_thread_) = false;
+ rtc::CopyOnWriteBuffer send_packet_ RTC_GUARDED_BY(network_thread_);
+ absl::optional<rtc::NetworkRoute> network_route_
+ RTC_GUARDED_BY(network_thread_);
+ std::map<rtc::Socket::Option, int> socket_options_
+ RTC_GUARDED_BY(network_thread_);
+ rtc::CopyOnWriteBuffer last_sent_packet_ RTC_GUARDED_BY(network_thread_);
rtc::Thread* const network_thread_;
};
diff --git a/pc/channel_manager_unittest.cc b/pc/channel_manager_unittest.cc
index 610d797..c0dddd8 100644
--- a/pc/channel_manager_unittest.cc
+++ b/pc/channel_manager_unittest.cc
@@ -62,16 +62,6 @@
fme_->SetVideoCodecs(MAKE_VECTOR(kVideoCodecs));
}
- std::unique_ptr<webrtc::RtpTransportInternal> CreateDtlsSrtpTransport() {
- rtp_dtls_transport_ = std::make_unique<FakeDtlsTransport>(
- "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP);
- auto dtls_srtp_transport = std::make_unique<webrtc::DtlsSrtpTransport>(
- /*rtcp_mux_required=*/true);
- dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport_.get(),
- /*rtcp_dtls_transport=*/nullptr);
- return dtls_srtp_transport;
- }
-
void TestCreateDestroyChannels(webrtc::RtpTransportInternal* rtp_transport) {
cricket::VoiceChannel* voice_channel = cm_->CreateVoiceChannel(
&fake_call_, cricket::MediaConfig(), rtp_transport,
@@ -95,7 +85,6 @@
cm_->Terminate();
}
- std::unique_ptr<DtlsTransportInternal> rtp_dtls_transport_;
std::unique_ptr<rtc::Thread> network_;
std::unique_ptr<rtc::Thread> worker_;
std::unique_ptr<webrtc::VideoBitrateAllocatorFactory>
@@ -178,8 +167,13 @@
TEST_F(ChannelManagerTest, CreateDestroyChannels) {
EXPECT_TRUE(cm_->Init());
- auto rtp_transport = CreateDtlsSrtpTransport();
- TestCreateDestroyChannels(rtp_transport.get());
+ auto rtp_dtls_transport = std::make_unique<FakeDtlsTransport>(
+ "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP);
+ auto dtls_srtp_transport = std::make_unique<webrtc::DtlsSrtpTransport>(
+ /*rtcp_mux_required=*/true);
+ dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport.get(),
+ /*rtcp_dtls_transport=*/nullptr);
+ TestCreateDestroyChannels(dtls_srtp_transport.get());
}
TEST_F(ChannelManagerTest, CreateDestroyChannelsOnThread) {
@@ -188,8 +182,17 @@
EXPECT_TRUE(cm_->set_worker_thread(worker_.get()));
EXPECT_TRUE(cm_->set_network_thread(network_.get()));
EXPECT_TRUE(cm_->Init());
- auto rtp_transport = CreateDtlsSrtpTransport();
- TestCreateDestroyChannels(rtp_transport.get());
+ auto rtp_dtls_transport = std::make_unique<FakeDtlsTransport>(
+ "fake_dtls_transport", cricket::ICE_CANDIDATE_COMPONENT_RTP,
+ network_.get());
+ auto dtls_srtp_transport = std::make_unique<webrtc::DtlsSrtpTransport>(
+ /*rtcp_mux_required=*/true);
+ network_->Invoke<void>(
+ RTC_FROM_HERE, [&rtp_dtls_transport, &dtls_srtp_transport] {
+ dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport.get(),
+ /*rtcp_dtls_transport=*/nullptr);
+ });
+ TestCreateDestroyChannels(dtls_srtp_transport.get());
}
} // namespace cricket
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index 4a0a6b4..ea4e828 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -170,11 +170,12 @@
} else {
// Confirmed to work with KT_RSA and KT_ECDSA.
fake_rtp_dtls_transport1_.reset(new cricket::FakeDtlsTransport(
- "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_));
rtp1 = fake_rtp_dtls_transport1_.get();
if (!(flags1 & RTCP_MUX)) {
fake_rtcp_dtls_transport1_.reset(new cricket::FakeDtlsTransport(
- "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTCP));
+ "channel1", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
+ network_thread_));
rtcp1 = fake_rtcp_dtls_transport1_.get();
}
if (flags1 & DTLS) {
@@ -199,11 +200,12 @@
} else {
// Confirmed to work with KT_RSA and KT_ECDSA.
fake_rtp_dtls_transport2_.reset(new cricket::FakeDtlsTransport(
- "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTP, network_thread_));
rtp2 = fake_rtp_dtls_transport2_.get();
if (!(flags2 & RTCP_MUX)) {
fake_rtcp_dtls_transport2_.reset(new cricket::FakeDtlsTransport(
- "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTCP));
+ "channel2", cricket::ICE_CANDIDATE_COMPONENT_RTCP,
+ network_thread_));
rtcp2 = fake_rtcp_dtls_transport2_.get();
}
if (flags2 & DTLS) {
@@ -284,10 +286,14 @@
auto rtp_transport = std::make_unique<webrtc::RtpTransport>(
rtcp_packet_transport == nullptr);
- rtp_transport->SetRtpPacketTransport(rtp_packet_transport);
- if (rtcp_packet_transport) {
- rtp_transport->SetRtcpPacketTransport(rtcp_packet_transport);
- }
+ network_thread_->Invoke<void>(
+ RTC_FROM_HERE,
+ [&rtp_transport, rtp_packet_transport, rtcp_packet_transport] {
+ rtp_transport->SetRtpPacketTransport(rtp_packet_transport);
+ if (rtcp_packet_transport) {
+ rtp_transport->SetRtcpPacketTransport(rtcp_packet_transport);
+ }
+ });
return rtp_transport;
}
@@ -297,8 +303,12 @@
auto dtls_srtp_transport = std::make_unique<webrtc::DtlsSrtpTransport>(
rtcp_dtls_transport == nullptr);
- dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport,
- rtcp_dtls_transport);
+ network_thread_->Invoke<void>(
+ RTC_FROM_HERE,
+ [&dtls_srtp_transport, rtp_dtls_transport, rtcp_dtls_transport] {
+ dtls_srtp_transport->SetDtlsTransports(rtp_dtls_transport,
+ rtcp_dtls_transport);
+ });
return dtls_srtp_transport;
}
@@ -1268,13 +1278,19 @@
fake_rtp_dtls_transport2_.get(), fake_rtcp_dtls_transport2_.get());
channel1_->SetRtpTransport(new_rtp_transport_.get());
- int option_val;
- ASSERT_TRUE(fake_rtp_dtls_transport2_->GetOption(
- rtc::Socket::Option::OPT_SNDBUF, &option_val));
- EXPECT_EQ(kSndBufSize, option_val);
- ASSERT_TRUE(fake_rtp_dtls_transport2_->GetOption(
- rtc::Socket::Option::OPT_RCVBUF, &option_val));
- EXPECT_EQ(kRcvBufSize, option_val);
+ bool rcv_success, send_success;
+ int rcv_buf, send_buf;
+ network_thread_->Invoke<void>(RTC_FROM_HERE, [&] {
+ send_success = fake_rtp_dtls_transport2_->GetOption(
+ rtc::Socket::Option::OPT_SNDBUF, &send_buf);
+ rcv_success = fake_rtp_dtls_transport2_->GetOption(
+ rtc::Socket::Option::OPT_RCVBUF, &rcv_buf);
+ });
+
+ ASSERT_TRUE(send_success);
+ EXPECT_EQ(kSndBufSize, send_buf);
+ ASSERT_TRUE(rcv_success);
+ EXPECT_EQ(kRcvBufSize, rcv_buf);
}
void CreateSimulcastContent(const std::vector<std::string>& rids,
