Allow DtlsTransport::Information() to be called off-thread
Bug: chromium:907849
Change-Id: I7e89aa21f26cbd858fa9845375681e5e6781fece
Reviewed-on: https://webrtc-review.googlesource.com/c/122503
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26686}diff --git a/api/dtls_transport_interface.h b/api/dtls_transport_interface.h
index ff3b107..7f35a23 100644
--- a/api/dtls_transport_interface.h
+++ b/api/dtls_transport_interface.h
@@ -55,15 +55,16 @@
};
// A DTLS transport, as represented to the outside world.
-// This object is created on the signaling thread, and can only be
-// accessed on that thread.
+// This object is created on the network thread, and can only be
+// accessed on that thread, except for functions explicitly marked otherwise.
// References can be held by other threads, and destruction can therefore
// be initiated by other threads.
class DtlsTransportInterface : public rtc::RefCountInterface {
public:
// Returns a pointer to the ICE transport that is owned by the DTLS transport.
virtual rtc::scoped_refptr<IceTransportInterface> ice_transport() = 0;
- // These functions can only be called from the signalling thread.
+ // Returns information on the state of the DtlsTransport.
+ // This function can be called from other threads.
virtual DtlsTransportInformation Information() = 0;
// Observer management.
virtual void RegisterObserver(DtlsTransportObserverInterface* observer) = 0;
diff --git a/pc/dtls_transport.cc b/pc/dtls_transport.cc
index 62a75f2..66e2787 100644
--- a/pc/dtls_transport.cc
+++ b/pc/dtls_transport.cc
@@ -43,69 +43,86 @@
// Implementation of DtlsTransportInterface
DtlsTransport::DtlsTransport(
std::unique_ptr<cricket::DtlsTransportInternal> internal)
- : signaling_thread_(rtc::Thread::Current()),
+ : owner_thread_(rtc::Thread::Current()),
+ info_(DtlsTransportState::kNew),
internal_dtls_transport_(std::move(internal)) {
RTC_DCHECK(internal_dtls_transport_.get());
internal_dtls_transport_->SignalDtlsState.connect(
this, &DtlsTransport::OnInternalDtlsState);
ice_transport_ = new rtc::RefCountedObject<IceTransportWithPointer>(
internal_dtls_transport_->ice_transport());
+ UpdateInformation();
}
DtlsTransport::~DtlsTransport() {
// We depend on the signaling thread to call Clear() before dropping
// its last reference to this object.
- RTC_DCHECK(signaling_thread_->IsCurrent() || !internal_dtls_transport_);
+ RTC_DCHECK(owner_thread_->IsCurrent() || !internal_dtls_transport_);
}
DtlsTransportInformation DtlsTransport::Information() {
- RTC_DCHECK(signaling_thread_->IsCurrent());
- if (internal()) {
- return DtlsTransportInformation(TranslateState(internal()->dtls_state()));
- } else {
- return DtlsTransportInformation(DtlsTransportState::kClosed);
- }
+ rtc::CritScope scope(&lock_);
+ return info_;
}
void DtlsTransport::RegisterObserver(DtlsTransportObserverInterface* observer) {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK_RUN_ON(owner_thread_);
RTC_DCHECK(observer);
observer_ = observer;
}
void DtlsTransport::UnregisterObserver() {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK_RUN_ON(owner_thread_);
observer_ = nullptr;
}
rtc::scoped_refptr<IceTransportInterface> DtlsTransport::ice_transport() {
+ RTC_DCHECK_RUN_ON(owner_thread_);
+ rtc::CritScope scope(&lock_);
return ice_transport_;
}
// Internal functions
void DtlsTransport::Clear() {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK_RUN_ON(owner_thread_);
RTC_DCHECK(internal());
- if (internal()->dtls_state() != cricket::DTLS_TRANSPORT_CLOSED) {
- internal_dtls_transport_.reset();
- if (observer_) {
- observer_->OnStateChange(Information());
- }
- } else {
- internal_dtls_transport_.reset();
+ bool must_send_event =
+ (internal()->dtls_state() != cricket::DTLS_TRANSPORT_CLOSED);
+ // The destructor of cricket::DtlsTransportInternal calls back
+ // into DtlsTransport, so we can't hold the lock while releasing.
+ std::unique_ptr<cricket::DtlsTransportInternal> transport_to_release;
+ {
+ rtc::CritScope scope(&lock_);
+ transport_to_release = std::move(internal_dtls_transport_);
+ ice_transport_->Clear();
}
- ice_transport_->Clear();
+ UpdateInformation();
+ if (observer_ && must_send_event) {
+ observer_->OnStateChange(Information());
+ }
}
void DtlsTransport::OnInternalDtlsState(
cricket::DtlsTransportInternal* transport,
cricket::DtlsTransportState state) {
- RTC_DCHECK(signaling_thread_->IsCurrent());
+ RTC_DCHECK_RUN_ON(owner_thread_);
RTC_DCHECK(transport == internal());
RTC_DCHECK(state == internal()->dtls_state());
+ UpdateInformation();
if (observer_) {
observer_->OnStateChange(Information());
}
}
+void DtlsTransport::UpdateInformation() {
+ RTC_DCHECK_RUN_ON(owner_thread_);
+ rtc::CritScope scope(&lock_);
+ if (internal_dtls_transport_) {
+ info_ = DtlsTransportInformation(
+ TranslateState(internal_dtls_transport_->dtls_state()));
+ } else {
+ info_ = DtlsTransportInformation(DtlsTransportState::kClosed);
+ }
+}
+
} // namespace webrtc
diff --git a/pc/dtls_transport.h b/pc/dtls_transport.h
index 535c7eb..0251716 100644
--- a/pc/dtls_transport.h
+++ b/pc/dtls_transport.h
@@ -28,7 +28,11 @@
class DtlsTransport : public DtlsTransportInterface,
public sigslot::has_slots<> {
public:
- // This object must be constructed on the signaling thread.
+ // This object must be constructed and updated on a consistent thread,
+ // the same thread as the one the cricket::DtlsTransportInternal object
+ // lives on.
+ // The Information() function can be called from a different thread,
+ // such as the signalling thread.
explicit DtlsTransport(
std::unique_ptr<cricket::DtlsTransportInternal> internal);
@@ -39,10 +43,12 @@
void Clear();
cricket::DtlsTransportInternal* internal() {
+ rtc::CritScope scope(&lock_);
return internal_dtls_transport_.get();
}
const cricket::DtlsTransportInternal* internal() const {
+ rtc::CritScope scope(&lock_);
return internal_dtls_transport_.get();
}
@@ -52,11 +58,16 @@
private:
void OnInternalDtlsState(cricket::DtlsTransportInternal* transport,
cricket::DtlsTransportState state);
+ void UpdateInformation();
DtlsTransportObserverInterface* observer_ = nullptr;
- rtc::Thread* signaling_thread_;
- std::unique_ptr<cricket::DtlsTransportInternal> internal_dtls_transport_;
- rtc::scoped_refptr<IceTransportWithPointer> ice_transport_;
+ rtc::Thread* owner_thread_;
+ rtc::CriticalSection lock_;
+ DtlsTransportInformation info_ RTC_GUARDED_BY(lock_);
+ std::unique_ptr<cricket::DtlsTransportInternal> internal_dtls_transport_
+ RTC_GUARDED_BY(lock_);
+ rtc::scoped_refptr<IceTransportWithPointer> ice_transport_
+ RTC_GUARDED_BY(lock_);
};
} // namespace webrtc
