diff --git a/pc/dtls_transport.cc b/pc/dtls_transport.cc index 15eed9e47b..4888d9f9e7 100644 --- a/pc/dtls_transport.cc +++ b/pc/dtls_transport.cc @@ -41,8 +41,18 @@ DtlsTransport::DtlsTransport( } DtlsTransport::~DtlsTransport() { + // TODO(tommi): Due to a reference being held by the RtpSenderBase + // implementation, the last reference to the `DtlsTransport` instance can + // be released on the signaling thread. + // RTC_DCHECK_RUN_ON(owner_thread_); + // We depend on the signaling thread to call Clear() before dropping // its last reference to this object. + + // If there are non `owner_thread_` references outstanding, and those + // references are the last ones released, we depend on Clear() having been + // called from the owner_thread before the last reference is deleted. + // `Clear()` is currently called from `JsepTransport::~JsepTransport`. RTC_DCHECK(owner_thread_->IsCurrent() || !internal_dtls_transport_); } @@ -72,14 +82,8 @@ void DtlsTransport::Clear() { RTC_DCHECK(internal()); bool must_send_event = (internal()->dtls_state() != DtlsTransportState::kClosed); - // The destructor of cricket::DtlsTransportInternal calls back - // into DtlsTransport, so we can't hold the lock while releasing. - std::unique_ptr transport_to_release; - { - MutexLock lock(&lock_); - transport_to_release = std::move(internal_dtls_transport_); - ice_transport_->Clear(); - } + internal_dtls_transport_.reset(); + ice_transport_->Clear(); UpdateInformation(); if (observer_ && must_send_event) { observer_->OnStateChange(Information()); @@ -100,7 +104,6 @@ void DtlsTransport::OnInternalDtlsState( void DtlsTransport::UpdateInformation() { RTC_DCHECK_RUN_ON(owner_thread_); - MutexLock lock(&lock_); if (internal_dtls_transport_) { if (internal_dtls_transport_->dtls_state() == DtlsTransportState::kConnected) { @@ -125,23 +128,24 @@ void DtlsTransport::UpdateInformation() { success &= internal_dtls_transport_->GetSslCipherSuite(&ssl_cipher_suite); success &= internal_dtls_transport_->GetSrtpCryptoSuite(&srtp_cipher); if (success) { - info_ = DtlsTransportInformation( + set_info(DtlsTransportInformation( internal_dtls_transport_->dtls_state(), role, tls_version, ssl_cipher_suite, srtp_cipher, - internal_dtls_transport_->GetRemoteSSLCertChain()); + internal_dtls_transport_->GetRemoteSSLCertChain())); } else { RTC_LOG(LS_ERROR) << "DtlsTransport in connected state has incomplete " "TLS information"; - info_ = DtlsTransportInformation( + set_info(DtlsTransportInformation( internal_dtls_transport_->dtls_state(), role, absl::nullopt, absl::nullopt, absl::nullopt, - internal_dtls_transport_->GetRemoteSSLCertChain()); + internal_dtls_transport_->GetRemoteSSLCertChain())); } } else { - info_ = DtlsTransportInformation(internal_dtls_transport_->dtls_state()); + set_info( + DtlsTransportInformation(internal_dtls_transport_->dtls_state())); } } else { - info_ = DtlsTransportInformation(DtlsTransportState::kClosed); + set_info(DtlsTransportInformation(DtlsTransportState::kClosed)); } } diff --git a/pc/dtls_transport.h b/pc/dtls_transport.h index cca4cc980a..a1893297e6 100644 --- a/pc/dtls_transport.h +++ b/pc/dtls_transport.h @@ -12,6 +12,7 @@ #define PC_DTLS_TRANSPORT_H_ #include +#include #include "api/dtls_transport_interface.h" #include "api/ice_transport_interface.h" @@ -40,18 +41,22 @@ class DtlsTransport : public DtlsTransportInterface { std::unique_ptr internal); rtc::scoped_refptr ice_transport() override; + + // Currently called from the signaling thread and potentially Chromium's + // JS thread. DtlsTransportInformation Information() override; + void RegisterObserver(DtlsTransportObserverInterface* observer) override; void UnregisterObserver() override; void Clear(); cricket::DtlsTransportInternal* internal() { - MutexLock lock(&lock_); + RTC_DCHECK_RUN_ON(owner_thread_); return internal_dtls_transport_.get(); } const cricket::DtlsTransportInternal* internal() const { - MutexLock lock(&lock_); + RTC_DCHECK_RUN_ON(owner_thread_); return internal_dtls_transport_.get(); } @@ -63,12 +68,19 @@ class DtlsTransport : public DtlsTransportInterface { DtlsTransportState state); void UpdateInformation(); + // Called when changing `info_`. We only change the values from the + // `owner_thread_` (a.k.a. the network thread). + void set_info(DtlsTransportInformation&& info) RTC_RUN_ON(owner_thread_) { + MutexLock lock(&lock_); + info_ = std::move(info); + } + DtlsTransportObserverInterface* observer_ = nullptr; rtc::Thread* owner_thread_; mutable Mutex lock_; DtlsTransportInformation info_ RTC_GUARDED_BY(lock_); std::unique_ptr internal_dtls_transport_ - RTC_GUARDED_BY(lock_); + RTC_GUARDED_BY(owner_thread_); const rtc::scoped_refptr ice_transport_; };