Fix race in BaseChannel destructor when network thread differ from worker thread

webrtc/pc/channel.cc

 /*
  *  Copyright 2004 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 165 matching lines @@
     RTC_DCHECK_EQ(network_thread, transport_controller->network_thread());
   }
   LOG(LS_INFO) << "Created channel for " << content_name;
 }
 
 BaseChannel::~BaseChannel() {
   TRACE_EVENT0("webrtc", "BaseChannel::~BaseChannel");
   ASSERT(worker_thread_ == rtc::Thread::Current());
   Deinit();
   StopConnectionMonitor();
-  // Send any outstanding RTCP packets.
-  network_thread_->Invoke<void>(Bind(&BaseChannel::FlushRtcpMessages_n, this));
   // Eats any outstanding messages or packets.
   worker_thread_->Clear(&invoker_);
   worker_thread_->Clear(this);
   // We must destroy the media channel before the transport channel, otherwise
   // the media channel may try to send on the dead transport channel. NULLing
   // is not an effective strategy since the sends will come on another thread.
   delete media_channel_;
   // Note that we don't just call SetTransportChannel_n(nullptr) because that
   // would call a pure virtual method which we can't do from a destructor.
-  network_thread_->Invoke<void>(Bind(&BaseChannel::DeinitNetwork_n, this));
+  network_thread_->Invoke<void>(
+      Bind(&BaseChannel::DestroyTransportChannels_n, this));
   LOG(LS_INFO) << "Destroyed channel";
 }
 
-void BaseChannel::DeinitNetwork_n() {
+void BaseChannel::DisconnectTransportChannels_n() {
+  // Send any outstanding RTCP packets.
+  FlushRtcpMessages_n();
+
+  // Stop signals from transport channels, but keep them alive because
+  // media_channel may use them from a different thread.
+  if (transport_channel_)
+    DisconnectFromTransportChannel(transport_channel_);
+  if (rtcp_transport_channel_)
+    DisconnectFromTransportChannel(rtcp_transport_channel_);
+
+  // Clear pending read packets/messages.
+  network_thread_->Clear(&invoker_);
+  network_thread_->Clear(this);
+}
+
+void BaseChannel::DestroyTransportChannels_n() {
   if (transport_channel_) {
-    DisconnectFromTransportChannel(transport_channel_);
     transport_controller_->DestroyTransportChannel_n(
         transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTP);
   }
   if (rtcp_transport_channel_) {
-    DisconnectFromTransportChannel(rtcp_transport_channel_);
     transport_controller_->DestroyTransportChannel_n(
         transport_name_, cricket::ICE_CANDIDATE_COMPONENT_RTCP);
   }
+  // Clear pending send packets/messages.
+  network_thread_->Clear(&invoker_);
   network_thread_->Clear(this);
 }
 
 bool BaseChannel::Init_w() {
   if (!network_thread_->Invoke<bool>(Bind(&BaseChannel::InitNetwork_n, this))) {
     return false;
   }
 
   // Both RTP and RTCP channels are set, we can call SetInterface on
   // media channel and it can set network options.
@@ skipping 14 matching lines @@
   if (rtcp_transport_enabled() &&
       !SetDtlsSrtpCryptoSuites_n(rtcp_transport_channel_, true)) {
     return false;
   }
   return true;
 }
 
 void BaseChannel::Deinit() {
   RTC_DCHECK(worker_thread_->IsCurrent());
   media_channel_->SetInterface(NULL);
+  // Packets arrive on network thread, processing packets calls virtual

[pthatcher1, 2016/05/13 18:33:27]

on network thread => on the network thread

+  // functions, so need to stop this process in Deinit that is called in
+  // derived classes destructor.
+  network_thread_->Invoke<void>(
+      Bind(&BaseChannel::DisconnectTransportChannels_n, this));
 }
 
 bool BaseChannel::SetTransport(const std::string& transport_name) {
   return network_thread_->Invoke<bool>(
       Bind(&BaseChannel::SetTransport_n, this, transport_name));
 }
 
 bool BaseChannel::SetTransport_n(const std::string& transport_name) {
   RTC_DCHECK(network_thread_->IsCurrent());
 
@@ skipping 2067 matching lines @@
   return data_channel_type_ == DCT_RTP && BaseChannel::ShouldSetupDtlsSrtp_n();
 }
 
 void DataChannel::OnStreamClosedRemotely(uint32_t sid) {
   rtc::TypedMessageData<uint32_t>* message =
       new rtc::TypedMessageData<uint32_t>(sid);
   signaling_thread()->Post(this, MSG_STREAMCLOSEDREMOTELY, message);
 }
 
 }  // namespace cricket