Remove accessor_lock_ in jsep_transport

Make access to rtcp_transport_ limited to network thread.

Bug: none
Change-Id: Id5c2834f758da595724079596d839e528c92e977
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/205982
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33180}
This commit is contained in:
Harald Alvestrand 2021-02-05 23:36:39 +00:00 committed by Commit Bot
parent 32edd54ea2
commit d4ad2ef732
2 changed files with 11 additions and 25 deletions

View File

@ -207,7 +207,6 @@ webrtc::RTCError JsepTransport::SetLocalJsepTransportDescription(
ice_parameters); ice_parameters);
{ {
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) { if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal()); RTC_DCHECK(rtcp_dtls_transport_->internal());
rtcp_dtls_transport_->internal()->ice_transport()->SetIceParameters( rtcp_dtls_transport_->internal()->ice_transport()->SetIceParameters(
@ -353,7 +352,6 @@ bool JsepTransport::GetStats(TransportStats* stats) {
bool ret = GetTransportStats(rtp_dtls_transport_->internal(), bool ret = GetTransportStats(rtp_dtls_transport_->internal(),
ICE_CANDIDATE_COMPONENT_RTP, stats); ICE_CANDIDATE_COMPONENT_RTP, stats);
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) { if (rtcp_dtls_transport_) {
RTC_DCHECK(rtcp_dtls_transport_->internal()); RTC_DCHECK(rtcp_dtls_transport_->internal());
ret &= GetTransportStats(rtcp_dtls_transport_->internal(), ret &= GetTransportStats(rtcp_dtls_transport_->internal(),
@ -466,8 +464,6 @@ bool JsepTransport::SetRtcpMux(bool enable,
} }
void JsepTransport::ActivateRtcpMux() { void JsepTransport::ActivateRtcpMux() {
RTC_DCHECK_RUN_ON(network_thread_);
if (unencrypted_rtp_transport_) { if (unencrypted_rtp_transport_) {
RTC_DCHECK(!sdes_transport_); RTC_DCHECK(!sdes_transport_);
RTC_DCHECK(!dtls_srtp_transport_); RTC_DCHECK(!dtls_srtp_transport_);
@ -483,10 +479,7 @@ void JsepTransport::ActivateRtcpMux() {
dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(), dtls_srtp_transport_->SetDtlsTransports(rtp_dtls_transport(),
/*rtcp_dtls_transport=*/nullptr); /*rtcp_dtls_transport=*/nullptr);
} }
{
webrtc::MutexLock lock(&accessor_lock_);
rtcp_dtls_transport_ = nullptr; // Destroy this reference. rtcp_dtls_transport_ = nullptr; // Destroy this reference.
}
// Notify the JsepTransportController to update the aggregate states. // Notify the JsepTransportController to update the aggregate states.
SignalRtcpMuxActive(); SignalRtcpMuxActive();
} }

View File

@ -46,7 +46,6 @@
#include "rtc_base/rtc_certificate.h" #include "rtc_base/rtc_certificate.h"
#include "rtc_base/ssl_fingerprint.h" #include "rtc_base/ssl_fingerprint.h"
#include "rtc_base/ssl_stream_adapter.h" #include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/synchronization/mutex.h"
#include "rtc_base/synchronization/sequence_checker.h" #include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/third_party/sigslot/sigslot.h"
#include "rtc_base/thread.h" #include "rtc_base/thread.h"
@ -125,7 +124,7 @@ class JsepTransport : public sigslot::has_slots<> {
webrtc::RTCError SetLocalJsepTransportDescription( webrtc::RTCError SetLocalJsepTransportDescription(
const JsepTransportDescription& jsep_description, const JsepTransportDescription& jsep_description,
webrtc::SdpType type) RTC_LOCKS_EXCLUDED(accessor_lock_); webrtc::SdpType type);
// Set the remote TransportDescription to be used by DTLS and ICE channels // Set the remote TransportDescription to be used by DTLS and ICE channels
// that are part of this Transport. // that are part of this Transport.
@ -154,7 +153,7 @@ class JsepTransport : public sigslot::has_slots<> {
absl::optional<rtc::SSLRole> GetDtlsRole() const; absl::optional<rtc::SSLRole> GetDtlsRole() const;
// TODO(deadbeef): Make this const. See comment in transportcontroller.h. // TODO(deadbeef): Make this const. See comment in transportcontroller.h.
bool GetStats(TransportStats* stats) RTC_LOCKS_EXCLUDED(accessor_lock_); bool GetStats(TransportStats* stats);
const JsepTransportDescription* local_description() const { const JsepTransportDescription* local_description() const {
RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK_RUN_ON(network_thread_);
@ -194,18 +193,16 @@ class JsepTransport : public sigslot::has_slots<> {
return nullptr; return nullptr;
} }
const DtlsTransportInternal* rtcp_dtls_transport() const const DtlsTransportInternal* rtcp_dtls_transport() const {
RTC_LOCKS_EXCLUDED(accessor_lock_) { RTC_DCHECK_RUN_ON(network_thread_);
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) { if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal(); return rtcp_dtls_transport_->internal();
} }
return nullptr; return nullptr;
} }
DtlsTransportInternal* rtcp_dtls_transport() DtlsTransportInternal* rtcp_dtls_transport() {
RTC_LOCKS_EXCLUDED(accessor_lock_) { RTC_DCHECK_RUN_ON(network_thread_);
webrtc::MutexLock lock(&accessor_lock_);
if (rtcp_dtls_transport_) { if (rtcp_dtls_transport_) {
return rtcp_dtls_transport_->internal(); return rtcp_dtls_transport_->internal();
} }
@ -249,7 +246,7 @@ class JsepTransport : public sigslot::has_slots<> {
private: private:
bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source); bool SetRtcpMux(bool enable, webrtc::SdpType type, ContentSource source);
void ActivateRtcpMux() RTC_LOCKS_EXCLUDED(accessor_lock_); void ActivateRtcpMux() RTC_RUN_ON(network_thread_);
bool SetSdes(const std::vector<CryptoParams>& cryptos, bool SetSdes(const std::vector<CryptoParams>& cryptos,
const std::vector<int>& encrypted_extension_ids, const std::vector<int>& encrypted_extension_ids,
@ -289,10 +286,6 @@ class JsepTransport : public sigslot::has_slots<> {
// Owning thread, for safety checks // Owning thread, for safety checks
const rtc::Thread* const network_thread_; const rtc::Thread* const network_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.
mutable webrtc::Mutex accessor_lock_;
const std::string mid_; const std::string mid_;
// needs-ice-restart bit as described in JSEP. // needs-ice-restart bit as described in JSEP.
bool needs_ice_restart_ RTC_GUARDED_BY(network_thread_) = false; bool needs_ice_restart_ RTC_GUARDED_BY(network_thread_) = false;
@ -315,10 +308,10 @@ class JsepTransport : public sigslot::has_slots<> {
const std::unique_ptr<webrtc::DtlsSrtpTransport> dtls_srtp_transport_; const std::unique_ptr<webrtc::DtlsSrtpTransport> dtls_srtp_transport_;
const rtc::scoped_refptr<webrtc::DtlsTransport> rtp_dtls_transport_; const rtc::scoped_refptr<webrtc::DtlsTransport> rtp_dtls_transport_;
// TODO(tommi): Restrict use to network thread, or make const. And delete the // The RTCP transport is const for all usages, except that it is cleared
// `accessor_lock_`. // when RTCP multiplexing is turned on; this happens on the network thread.
rtc::scoped_refptr<webrtc::DtlsTransport> rtcp_dtls_transport_ rtc::scoped_refptr<webrtc::DtlsTransport> rtcp_dtls_transport_
RTC_GUARDED_BY(accessor_lock_); RTC_GUARDED_BY(network_thread_);
const std::unique_ptr<webrtc::DataChannelTransportInterface> const std::unique_ptr<webrtc::DataChannelTransportInterface>
sctp_data_channel_transport_; sctp_data_channel_transport_;