JsepTransport: remove lock recursions.
This change removes lock recursions and adds thread annotations.
Bug: webrtc:11567
Change-Id: Iefe1875182b7f8f8df3e9bd02e964530389b0b3d
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/175123
Reviewed-by: Stefan Holmer <stefan@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#31296}
diff --git a/pc/jsep_transport.cc b/pc/jsep_transport.cc
index 5bf74f1..5788825 100644
--- a/pc/jsep_transport.cc
+++ b/pc/jsep_transport.cc
@@ -386,12 +386,12 @@
absl::optional<OpaqueTransportParameters>
JsepTransport::GetTransportParameters() const {
rtc::CritScope scope(&accessor_lock_);
- if (!datagram_transport()) {
+ if (!datagram_transport_) {
return absl::nullopt;
}
OpaqueTransportParameters params;
- params.parameters = datagram_transport()->GetTransportParameters();
+ params.parameters = datagram_transport_->GetTransportParameters();
return params;
}
@@ -462,7 +462,6 @@
DtlsTransportInternal* dtls_transport,
absl::optional<rtc::SSLRole> dtls_role,
rtc::SSLFingerprint* remote_fingerprint) {
- RTC_DCHECK_RUN_ON(network_thread_);
RTC_DCHECK(dtls_transport);
// Set SSL role. Role must be set before fingerprint is applied, which
// initiates DTLS setup.
@@ -535,7 +534,7 @@
RTC_DCHECK(dtls_srtp_transport_);
RTC_DCHECK(!unencrypted_rtp_transport_);
RTC_DCHECK(!sdes_transport_);
- dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(),
+ dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport_locked(),
/*rtcp_dtls_transport=*/nullptr);
}
rtcp_dtls_transport_ = nullptr; // Destroy this reference.
@@ -549,7 +548,6 @@
webrtc::SdpType type,
ContentSource source) {
RTC_DCHECK_RUN_ON(network_thread_);
- rtc::CritScope scope(&accessor_lock_);
bool ret = false;
ret = sdes_negotiator_.Process(cryptos, type, source);
if (!ret) {
@@ -734,7 +732,6 @@
bool JsepTransport::GetTransportStats(DtlsTransportInternal* dtls_transport,
TransportStats* stats) {
RTC_DCHECK_RUN_ON(network_thread_);
- rtc::CritScope scope(&accessor_lock_);
RTC_DCHECK(dtls_transport);
TransportChannelStats substats;
if (rtcp_dtls_transport_) {
diff --git a/pc/jsep_transport.h b/pc/jsep_transport.h
index 6d88def..2d20d29 100644
--- a/pc/jsep_transport.h
+++ b/pc/jsep_transport.h
@@ -128,14 +128,15 @@
webrtc::RTCError SetLocalJsepTransportDescription(
const JsepTransportDescription& jsep_description,
- webrtc::SdpType type);
+ webrtc::SdpType type) RTC_LOCKS_EXCLUDED(accessor_lock_);
// Set the remote TransportDescription to be used by DTLS and ICE channels
// that are part of this Transport.
webrtc::RTCError SetRemoteJsepTransportDescription(
const JsepTransportDescription& jsep_description,
- webrtc::SdpType type);
- webrtc::RTCError AddRemoteCandidates(const Candidates& candidates);
+ webrtc::SdpType type) RTC_LOCKS_EXCLUDED(accessor_lock_);
+ webrtc::RTCError AddRemoteCandidates(const Candidates& candidates)
+ RTC_LOCKS_EXCLUDED(accessor_lock_);
// Set the "needs-ice-restart" flag as described in JSEP. After the flag is
// set, offers should generate new ufrags/passwords until an ICE restart
@@ -143,23 +144,25 @@
//
// This and the below method can be called safely from any thread as long as
// SetXTransportDescription is not in progress.
- void SetNeedsIceRestartFlag();
+ void SetNeedsIceRestartFlag() RTC_LOCKS_EXCLUDED(accessor_lock_);
// Returns true if the ICE restart flag above was set, and no ICE restart has
// occurred yet for this transport (by applying a local description with
// changed ufrag/password).
- bool needs_ice_restart() const {
+ bool needs_ice_restart() const RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
return needs_ice_restart_;
}
// Returns role if negotiated, or empty absl::optional if it hasn't been
// negotiated yet.
- absl::optional<rtc::SSLRole> GetDtlsRole() const;
+ absl::optional<rtc::SSLRole> GetDtlsRole() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_);
- absl::optional<OpaqueTransportParameters> GetTransportParameters() const;
+ absl::optional<OpaqueTransportParameters> GetTransportParameters() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_);
// TODO(deadbeef): Make this const. See comment in transportcontroller.h.
- bool GetStats(TransportStats* stats);
+ bool GetStats(TransportStats* stats) RTC_LOCKS_EXCLUDED(accessor_lock_);
const JsepTransportDescription* local_description() const {
RTC_DCHECK_RUN_ON(network_thread_);
@@ -171,7 +174,8 @@
return remote_description_.get();
}
- webrtc::RtpTransportInternal* rtp_transport() const {
+ webrtc::RtpTransportInternal* rtp_transport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
if (composite_rtp_transport_) {
return composite_rtp_transport_.get();
@@ -182,7 +186,8 @@
}
}
- const DtlsTransportInternal* rtp_dtls_transport() const {
+ const DtlsTransportInternal* rtp_dtls_transport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
if (rtp_dtls_transport_) {
return rtp_dtls_transport_->internal();
@@ -191,16 +196,14 @@
}
}
- DtlsTransportInternal* rtp_dtls_transport() {
+ DtlsTransportInternal* rtp_dtls_transport()
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
- if (rtp_dtls_transport_) {
- return rtp_dtls_transport_->internal();
- } else {
- return nullptr;
- }
+ return rtp_dtls_transport_locked();
}
- const DtlsTransportInternal* rtcp_dtls_transport() const {
+ const DtlsTransportInternal* rtcp_dtls_transport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal();
@@ -209,7 +212,8 @@
}
}
- DtlsTransportInternal* rtcp_dtls_transport() {
+ DtlsTransportInternal* rtcp_dtls_transport()
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal();
@@ -218,17 +222,20 @@
}
}
- rtc::scoped_refptr<webrtc::DtlsTransport> RtpDtlsTransport() {
+ rtc::scoped_refptr<webrtc::DtlsTransport> RtpDtlsTransport()
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
return rtp_dtls_transport_;
}
- rtc::scoped_refptr<webrtc::SctpTransport> SctpTransport() const {
+ rtc::scoped_refptr<webrtc::SctpTransport> SctpTransport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
return sctp_transport_;
}
- webrtc::DataChannelTransportInterface* data_channel_transport() const {
+ webrtc::DataChannelTransportInterface* data_channel_transport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
if (composite_data_channel_transport_) {
return composite_data_channel_transport_.get();
@@ -239,7 +246,8 @@
}
// Returns datagram transport, if available.
- webrtc::DatagramTransportInterface* datagram_transport() const {
+ webrtc::DatagramTransportInterface* datagram_transport() const
+ RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
return datagram_transport_.get();
}
@@ -271,6 +279,15 @@
void SetActiveResetSrtpParams(bool active_reset_srtp_params);
private:
+ DtlsTransportInternal* rtp_dtls_transport_locked()
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_) {
+ if (rtp_dtls_transport_) {
+ return rtp_dtls_transport_->internal();
+ } else {
+ return nullptr;
+ }
+ }
+
bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source);
void ActivateRtcpMux();
@@ -278,7 +295,8 @@
bool SetSdes(const std::vector<CryptoParams>& cryptos,
const std::vector<int>& encrypted_extension_ids,
webrtc::SdpType type,
- ContentSource source);
+ ContentSource source)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_);
// Negotiates and sets the DTLS parameters based on the current local and
// remote transport description, such as the DTLS role to use, and whether
@@ -295,26 +313,28 @@
webrtc::SdpType local_description_type,
ConnectionRole local_connection_role,
ConnectionRole remote_connection_role,
- absl::optional<rtc::SSLRole>* negotiated_dtls_role);
+ absl::optional<rtc::SSLRole>* negotiated_dtls_role)
+ RTC_LOCKS_EXCLUDED(accessor_lock_);
// Pushes down the ICE parameters from the remote description.
void SetRemoteIceParameters(const IceParameters& ice_parameters,
IceTransportInternal* ice);
// Pushes down the DTLS parameters obtained via negotiation.
- webrtc::RTCError SetNegotiatedDtlsParameters(
+ static webrtc::RTCError SetNegotiatedDtlsParameters(
DtlsTransportInternal* dtls_transport,
absl::optional<rtc::SSLRole> dtls_role,
rtc::SSLFingerprint* remote_fingerprint);
bool GetTransportStats(DtlsTransportInternal* dtls_transport,
- TransportStats* stats);
+ TransportStats* stats)
+ RTC_EXCLUSIVE_LOCKS_REQUIRED(accessor_lock_);
// Deactivates, signals removal, and deletes |composite_rtp_transport_| if the
// current state of negotiation is sufficient to determine which rtp_transport
// and data channel transport to use.
void NegotiateDatagramTransport(webrtc::SdpType type)
- RTC_RUN_ON(network_thread_);
+ RTC_RUN_ON(network_thread_) RTC_LOCKS_EXCLUDED(accessor_lock_);
// Returns the default (non-datagram) rtp transport, if any.
webrtc::RtpTransportInternal* default_rtp_transport() const
