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}

api/dtls_transport_interface.h

@@ -55,15 +55,16 @@ class DtlsTransportObserverInterface {
 };
 
 // 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;

pc/dtls_transport.cc

@@ -43,69 +43,86 @@ DtlsTransportState TranslateState(cricket::DtlsTransportState internal_state) {
 // 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_) {
+  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();
+  }
+  UpdateInformation();
+  if (observer_ && must_send_event) {
     observer_->OnStateChange(Information());
   }
-  } else {
-    internal_dtls_transport_.reset();
-  }
-  ice_transport_->Clear();
 }
 
 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

pc/dtls_transport.h

@@ -28,7 +28,11 @@ class IceTransportWithPointer;
 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 @@ class DtlsTransport : public DtlsTransportInterface,
   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 @@ class DtlsTransport : public DtlsTransportInterface,
  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