From 2cc368fd7a398078d82ff9e4cc605bf851432077 Mon Sep 17 00:00:00 2001 From: Karl Wiberg Date: Tue, 2 Apr 2019 11:31:56 +0200 Subject: [PATCH] Add thread safety annotations for some more PeerConnection members (part 9) Plus all the annotations that were necessary to make things compile again. Bug: webrtc:9987 Change-Id: Ie958f4d86319e86527567ca1273a0595ccceee17 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130490 Commit-Queue: Karl Wiberg Reviewed-by: Steve Anton Cr-Commit-Position: refs/heads/master@{#27411} --- pc/peer_connection.cc | 14 +++++++++++--- pc/peer_connection.h | 36 +++++++++++++++++++++++++++--------- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 5b702e0c7a..a7fc8db6b3 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -3549,6 +3549,7 @@ bool PeerConnection::AddIceCandidate( bool PeerConnection::RemoveIceCandidates( const std::vector& candidates) { TRACE_EVENT0("webrtc", "PeerConnection::RemoveIceCandidates"); + RTC_DCHECK_RUN_ON(signaling_thread()); if (IsClosed()) { RTC_LOG(LS_ERROR) << "RemoveIceCandidates: PeerConnection is closed."; return false; @@ -3740,16 +3741,19 @@ void PeerConnection::StopRtcEventLog() { rtc::scoped_refptr PeerConnection::LookupDtlsTransportByMid(const std::string& mid) { + RTC_DCHECK_RUN_ON(signaling_thread()); return transport_controller_->LookupDtlsTransportByMid(mid); } rtc::scoped_refptr PeerConnection::LookupDtlsTransportByMidInternal(const std::string& mid) { + RTC_DCHECK_RUN_ON(signaling_thread()); return transport_controller_->LookupDtlsTransportByMid(mid); } rtc::scoped_refptr PeerConnection::GetSctpTransport() const { + RTC_DCHECK_RUN_ON(signaling_thread()); return sctp_transport_; } @@ -5474,6 +5478,7 @@ bool PeerConnection::GetSctpSslRole(rtc::SSLRole* role) { bool PeerConnection::GetSslRole(const std::string& content_name, rtc::SSLRole* role) { + RTC_DCHECK_RUN_ON(signaling_thread()); if (!local_description() || !remote_description()) { RTC_LOG(LS_INFO) << "Local and Remote descriptions must be applied to get the " @@ -5694,6 +5699,7 @@ cricket::IceConfig PeerConnection::ParseIceConfig( bool PeerConnection::SendData(const cricket::SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, cricket::SendDataResult* result) { + RTC_DCHECK_RUN_ON(signaling_thread()); if (!rtp_data_channel_ && !sctp_transport_ && !media_transport_) { RTC_LOG(LS_ERROR) << "SendData called when rtp_data_channel_, " "sctp_transport_, and media_transport_ are NULL."; @@ -5846,6 +5852,7 @@ void PeerConnection::OnChannelClosed(int channel_id) { } absl::optional PeerConnection::sctp_transport_name() const { + RTC_DCHECK_RUN_ON(signaling_thread()); if (sctp_mid_ && transport_controller_) { auto dtls_transport = transport_controller_->GetDtlsTransport(*sctp_mid_); if (dtls_transport) { @@ -5897,6 +5904,7 @@ PeerConnection::GetTransportStatsByNames( RTC_FROM_HERE, [&] { return GetTransportStatsByNames(transport_names); }); } + RTC_DCHECK_RUN_ON(network_thread()); std::map transport_stats_by_name; for (const std::string& transport_name : transport_names) { cricket::TransportStats transport_stats; @@ -6327,7 +6335,7 @@ Call::Stats PeerConnection::GetCallStats() { } bool PeerConnection::CreateSctpTransport_n(const std::string& mid) { - RTC_DCHECK(network_thread()->IsCurrent()); + RTC_DCHECK_RUN_ON(network_thread()); RTC_DCHECK(sctp_factory_); rtc::scoped_refptr webrtc_dtls_transport = transport_controller_->LookupDtlsTransportByMid(mid); @@ -6363,7 +6371,6 @@ void PeerConnection::DestroySctpTransport_n() { sctp_transport_ = nullptr; sctp_mid_.reset(); sctp_invoker_.reset(nullptr); - sctp_ready_to_send_data_ = false; } void PeerConnection::OnSctpTransportReadyToSendData_n() { @@ -6380,7 +6387,7 @@ void PeerConnection::OnSctpTransportReadyToSendData_n() { } void PeerConnection::OnSctpTransportReadyToSendData_s(bool ready) { - RTC_DCHECK(signaling_thread()->IsCurrent()); + RTC_DCHECK_RUN_ON(signaling_thread()); sctp_ready_to_send_data_ = ready; SignalSctpReadyToSendData(ready); } @@ -7006,6 +7013,7 @@ void PeerConnection::DestroyDataChannel() { OnDataChannelDestroyed(); network_thread()->Invoke(RTC_FROM_HERE, [this] { DestroySctpTransport_n(); }); + sctp_ready_to_send_data_ = false; } if (media_transport_) { diff --git a/pc/peer_connection.h b/pc/peer_connection.h index 30155eaf07..b584ebeeac 100644 --- a/pc/peer_connection.h +++ b/pc/peer_connection.h @@ -227,6 +227,7 @@ class PeerConnection : public PeerConnectionInternal, } bool initial_offerer() const override { + RTC_DCHECK_RUN_ON(signaling_thread()); return transport_controller_ && transport_controller_->initial_offerer(); } @@ -252,6 +253,7 @@ class PeerConnection : public PeerConnectionInternal, } absl::optional sctp_content_name() const override { + RTC_DCHECK_RUN_ON(signaling_thread()); return sctp_mid_; } @@ -616,7 +618,7 @@ class PeerConnection : public PeerConnectionInternal, // Returns the MID for the data section associated with either the // RtpDataChannel or SCTP data channel, if it has been set. If no data // channels are configured this will return nullopt. - absl::optional GetDataMid() const; + absl::optional GetDataMid() const RTC_RUN_ON(signaling_thread()); // Remove all local and remote senders of type |media_type|. // Called when a media type is rejected (m-line set to port 0). @@ -1053,7 +1055,8 @@ class PeerConnection : public PeerConnectionInternal, CryptoOptions GetCryptoOptions() RTC_RUN_ON(signaling_thread()); // Returns rtp transport, result can not be nullptr. - RtpTransportInternal* GetRtpTransport(const std::string& mid) { + RtpTransportInternal* GetRtpTransport(const std::string& mid) + RTC_RUN_ON(signaling_thread()) { auto rtp_transport = transport_controller_->GetRtpTransport(mid); RTC_DCHECK(rtp_transport); return rtp_transport; @@ -1177,7 +1180,9 @@ class PeerConnection : public PeerConnectionInternal, std::vector< rtc::scoped_refptr>> - transceivers_; + transceivers_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling + // and network thread. + // In Unified Plan, if we encounter remote SDP that does not contain an a=msid // line we create and use a stream with a random ID for our receivers. This is // to support legacy endpoints that do not support the a=msid attribute (as @@ -1194,21 +1199,34 @@ class PeerConnection : public PeerConnectionInternal, std::string session_id_ RTC_GUARDED_BY(signaling_thread()); - std::unique_ptr transport_controller_; - std::unique_ptr sctp_factory_; + std::unique_ptr + transport_controller_; // TODO(bugs.webrtc.org/9987): Accessed on both + // signaling and network thread. + std::unique_ptr + sctp_factory_; // TODO(bugs.webrtc.org/9987): Accessed on both + // signaling and network thread. // |rtp_data_channel_| is used if in RTP data channel mode, |sctp_transport_| // when using SCTP. - cricket::RtpDataChannel* rtp_data_channel_ = nullptr; + cricket::RtpDataChannel* rtp_data_channel_ = + nullptr; // TODO(bugs.webrtc.org/9987): Accessed on both + // signaling and some other thread. cricket::SctpTransportInternal* cricket_sctp_transport() { return sctp_transport_->internal(); } - rtc::scoped_refptr sctp_transport_; + rtc::scoped_refptr + sctp_transport_; // TODO(bugs.webrtc.org/9987): Accessed on both + // signaling and network thread. + // |sctp_mid_| is the content name (MID) in SDP. - absl::optional sctp_mid_; + absl::optional + sctp_mid_; // TODO(bugs.webrtc.org/9987): Accessed on both signaling + // and network thread. + // Value cached on signaling thread. Only updated when SctpReadyToSendData // fires on the signaling thread. - bool sctp_ready_to_send_data_ = false; + bool sctp_ready_to_send_data_ RTC_GUARDED_BY(signaling_thread()) = false; + // Same as signals provided by SctpTransport, but these are guaranteed to // fire on the signaling thread, whereas SctpTransport fires on the networking // thread.