admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Delete use of RecursiveCriticalSection in JsepTransport

Browse Source 
Mark corresponding webrtc::Mutex as mutable, to allow use in const methods.

Bug: webrtc:11567
Change-Id: Ia8c731a91c719a531799abf24fd30a15b54428af
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/204801
Reviewed-by: Markus Handell <handellm@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33126}
This commit is contained in:
Niels Möller 2021-02-01 12:19:19 +01:00 committed by Commit Bot
parent 51e5c4b0f4
commit 78f87ab106
2 changed files with 27 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

18
pc/jsep_transport.cc
View File

@ -176,7 +176,7 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
// If doing SDES, setup the SDES crypto parameters.
{
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (sdes_transport_) {
RTC_DCHECK(!unencrypted_rtp_transport_);
RTC_DCHECK(!dtls_srtp_transport_);
@ -213,7 +213,7 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
}
}
{
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
RTC_DCHECK(rtp_dtls_transport_->internal());
rtp_dtls_transport_->internal()->ice_transport()->SetIceParameters(
ice_parameters);
@ -233,7 +233,7 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
return error;
}
{
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (needs_ice_restart_ && ice_restarting) {
needs_ice_restart_ = false;
RTC_LOG(LS_VERBOSE) << "needs-ice-restart flag cleared for transport "
@ -270,7 +270,7 @@ webrtc::RTCError JsepTransport::SetRemoteJsepTransportDescription(
// If doing SDES, setup the SDES crypto parameters.
{
rtc::CritScope lock(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (sdes_transport_) {
RTC_DCHECK(!unencrypted_rtp_transport_);
RTC_DCHECK(!dtls_srtp_transport_);
@ -341,7 +341,7 @@ webrtc::RTCError JsepTransport::AddRemoteCandidates(
}
void JsepTransport::SetNeedsIceRestartFlag() {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (!needs_ice_restart_) {
needs_ice_restart_ = true;
RTC_LOG(LS_VERBOSE) << "needs-ice-restart flag set for transport " << mid();
@ -350,7 +350,7 @@ void JsepTransport::SetNeedsIceRestartFlag() {
absl::optional<rtc::SSLRole> JsepTransport::GetDtlsRole() const {
RTC_DCHECK_RUN_ON(network_thread_);
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
RTC_DCHECK(rtp_dtls_transport_);
RTC_DCHECK(rtp_dtls_transport_->internal());
rtc::SSLRole dtls_role;
@ -363,7 +363,7 @@ absl::optional<rtc::SSLRole> JsepTransport::GetDtlsRole() const {
bool JsepTransport::GetStats(TransportStats* stats) {
RTC_DCHECK_RUN_ON(network_thread_);
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
stats->transport_name = mid();
stats->channel_stats.clear();
RTC_DCHECK(rtp_dtls_transport_->internal());
@ -405,7 +405,7 @@ webrtc::RTCError JsepTransport::VerifyCertificateFingerprint(
void JsepTransport::SetActiveResetSrtpParams(bool active_reset_srtp_params) {
RTC_DCHECK_RUN_ON(network_thread_);
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (dtls_srtp_transport_) {
RTC_LOG(INFO)
<< "Setting active_reset_srtp_params of DtlsSrtpTransport to: "
@ -487,7 +487,7 @@ void JsepTransport::ActivateRtcpMux() {
RTC_DCHECK_RUN_ON(network_thread_);
}
{
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (unencrypted_rtp_transport_) {
RTC_DCHECK(!sdes_transport_);
RTC_DCHECK(!dtls_srtp_transport_);

32
pc/jsep_transport.h
View File

@ -44,10 +44,10 @@
#include "pc/transport_stats.h"
#include "rtc_base/checks.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/deprecated/recursive_critical_section.h"
#include "rtc_base/rtc_certificate.h"
#include "rtc_base/ssl_fingerprint.h"
#include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
#include "rtc_base/thread.h"
@ -144,12 +144,14 @@ class JsepTransport : public sigslot::has_slots<> {
//
// This and the below method can be called safely from any thread as long as
// SetXTransportDescription is not in progress.
// TODO(tommi): Investigate on which threads (network or signal?) we really
// need to access the needs_ice_restart flag.
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 RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
return needs_ice_restart_;
}
@ -173,7 +175,7 @@ class JsepTransport : public sigslot::has_slots<> {
webrtc::RtpTransportInternal* rtp_transport() const
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (composite_rtp_transport_) {
return composite_rtp_transport_.get();
} else if (datagram_rtp_transport_) {
@ -185,7 +187,7 @@ class JsepTransport : public sigslot::has_slots<> {
const DtlsTransportInternal* rtp_dtls_transport() const
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (rtp_dtls_transport_) {
return rtp_dtls_transport_->internal();
} else {
@ -195,13 +197,13 @@ class JsepTransport : public sigslot::has_slots<> {
DtlsTransportInternal* rtp_dtls_transport()
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
return rtp_dtls_transport_locked();
}
const DtlsTransportInternal* rtcp_dtls_transport() const
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal();
} else {
@ -211,7 +213,7 @@ class JsepTransport : public sigslot::has_slots<> {
DtlsTransportInternal* rtcp_dtls_transport()
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal();
} else {
@ -221,13 +223,13 @@ class JsepTransport : public sigslot::has_slots<> {
rtc::scoped_refptr<webrtc::DtlsTransport> RtpDtlsTransport()
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
return rtp_dtls_transport_;
}
rtc::scoped_refptr<webrtc::SctpTransport> SctpTransport() const
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
return sctp_transport_;
}
@ -235,7 +237,7 @@ class JsepTransport : public sigslot::has_slots<> {
// SctpTransport() instead.
webrtc::DataChannelTransportInterface* data_channel_transport() const
RTC_LOCKS_EXCLUDED(accessor_lock_) {
rtc::CritScope scope(&accessor_lock_);
webrtc::MutexLock lock(&accessor_lock_);
if (sctp_data_channel_transport_) {
return sctp_data_channel_transport_.get();
}
@ -257,7 +259,8 @@ class JsepTransport : public sigslot::has_slots<> {
const rtc::RTCCertificate* certificate,
const rtc::SSLFingerprint* fingerprint) const;
void SetActiveResetSrtpParams(bool active_reset_srtp_params);
void SetActiveResetSrtpParams(bool active_reset_srtp_params)
RTC_LOCKS_EXCLUDED(accessor_lock_);
private:
DtlsTransportInternal* rtp_dtls_transport_locked()
@ -271,7 +274,7 @@ class JsepTransport : public sigslot::has_slots<> {
bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source);
void ActivateRtcpMux();
void ActivateRtcpMux() RTC_LOCKS_EXCLUDED(accessor_lock_);
bool SetSdes(const std::vector<CryptoParams>& cryptos,
const std::vector<int>& encrypted_extension_ids,
@ -327,9 +330,10 @@ class JsepTransport : public sigslot::has_slots<> {
// Owning thread, for safety checks
const rtc::Thread* const network_thread_;
// Critical scope for fields accessed off-thread
// Critical scope for fields accessed off-thread. Mutable, since it is used by
// getter methods.
// TODO(https://bugs.webrtc.org/10300): Stop doing this.
rtc::RecursiveCriticalSection accessor_lock_;
mutable webrtc::Mutex accessor_lock_;
const std::string mid_;
// needs-ice-restart bit as described in JSEP.
bool needs_ice_restart_ RTC_GUARDED_BY(accessor_lock_) = false;