Bug 1411977 - Part 2: Stop using sync dispatch and queue jumping with SingletonThreadHolder. r?drno draft
authorByron Campen [:bwc] <docfaraday@gmail.com>
Thu, 25 Jan 2018 09:53:04 -0600
changeset 748500 1948b816257188f5c71d8b9a44a6aad2aa8c6a57
parent 747305 99a14922a3e8ee34a914669b2c0572e28e8b7286
child 748501 4780c1c84e8e99b7a882cd952d56eda48f31b5a1
push id97190
push userbcampen@mozilla.com
push dateMon, 29 Jan 2018 22:41:18 +0000
reviewersdrno
bugs1411977
milestone60.0a1
Bug 1411977 - Part 2: Stop using sync dispatch and queue jumping with SingletonThreadHolder. r?drno MozReview-Commit-ID: F2BbHI2kiK1
media/mtransport/nr_socket_prsock.cpp
media/mtransport/nr_socket_prsock.h
--- a/media/mtransport/nr_socket_prsock.cpp
+++ b/media/mtransport/nr_socket_prsock.cpp
@@ -203,49 +203,34 @@ public:
   nsIThread* GetThread() {
     return mThread;
   }
 
   /*
    * Keep track of how many instances are using a SingletonThreadHolder.
    * When no one is using it, shut it down
    */
-  void AddUse()
+ void AddUse()
   {
-    RUN_ON_THREAD(mParentThread,
-                  mozilla::WrapRunnable(RefPtr<SingletonThreadHolder>(this),
-                                        &SingletonThreadHolder::AddUse_i),
-                  NS_DISPATCH_SYNC);
-  }
-
-  void AddUse_i()
-  {
+    MOZ_ASSERT(mParentThread == NS_GetCurrentThread());
     MOZ_ASSERT(int32_t(mUseCount) >= 0, "illegal refcnt");
     nsrefcnt count = ++mUseCount;
     if (count == 1) {
       // idle -> in-use
       nsresult rv = NS_NewNamedThread(mName, getter_AddRefs(mThread));
       MOZ_RELEASE_ASSERT(NS_SUCCEEDED(rv) && mThread,
                          "Should successfully create mtransport I/O thread");
       r_log(LOG_GENERIC,LOG_DEBUG,"Created wrapped SingletonThread %p",
             mThread.get());
     }
     r_log(LOG_GENERIC,LOG_DEBUG,"AddUse_i: %lu", (unsigned long) count);
   }
 
   void ReleaseUse()
   {
-    RUN_ON_THREAD(mParentThread,
-                  mozilla::WrapRunnable(RefPtr<SingletonThreadHolder>(this),
-                                        &SingletonThreadHolder::ReleaseUse_i),
-                  NS_DISPATCH_SYNC);
-  }
-
-  void ReleaseUse_i()
-  {
     MOZ_ASSERT(mParentThread == NS_GetCurrentThread());
     nsrefcnt count = --mUseCount;
     MOZ_ASSERT(int32_t(mUseCount) >= 0, "illegal refcnt");
     if (mThread && count == 0) {
       // in-use -> idle -- no one forcing it to remain instantiated
       r_log(LOG_GENERIC,LOG_DEBUG,"Shutting down wrapped SingletonThread %p",
             mThread.get());
       mThread->Shutdown();
@@ -1139,28 +1124,25 @@ NrUdpSocketIpc::NrUdpSocketIpc()
   : NrSocketIpc(GetIOThreadAndAddUse_s()),
     monitor_("NrUdpSocketIpc"),
     err_(false),
     state_(NR_INIT) {
 }
 
 NrUdpSocketIpc::~NrUdpSocketIpc()
 {
-  // also guarantees socket_child_ is released from the io_thread, and
-  // tells the SingletonThreadHolder we're done with it
-
 #if defined(MOZILLA_INTERNAL_API)
   // close(), but transfer the socket_child_ reference to die as well
+  // destroy_i also dispatches back to STS to call ReleaseUse, to avoid shutting
+  // down the IO thread before close() runs.
   RUN_ON_THREAD(io_thread_,
-                mozilla::WrapRunnableNM(&NrUdpSocketIpc::release_child_i,
-                                        socket_child_.forget().take()),
+                mozilla::WrapRunnableNM(&NrUdpSocketIpc::destroy_i,
+                                        socket_child_.forget().take(),
+                                        sts_thread_),
                 NS_DISPATCH_NORMAL);
-  // This may shut down the io_thread_, but it should spin the event loop so
-  // the above runnable happens.
-  sThread->ReleaseUse();
 #endif
 }
 
 // IUDPSocketInternal interfaces
 // callback while error happened in UDP socket operation
 NS_IMETHODIMP NrUdpSocketIpc::CallListenerError(const nsACString &message,
                                                 const nsACString &filename,
                                                 uint32_t line_number) {
@@ -1342,16 +1324,17 @@ int NrUdpSocketIpc::create(nr_transport_
 
   // wildcard address will be resolved at NrUdpSocketIpc::CallListenerVoid
   if ((r=nr_transport_addr_copy(&my_addr_, addr))) {
     ABORT(r);
   }
 
   state_ = NR_CONNECTING;
 
+  MOZ_ASSERT(io_thread_);
   RUN_ON_THREAD(io_thread_,
                 mozilla::WrapRunnable(RefPtr<NrUdpSocketIpc>(this),
                                       &NrUdpSocketIpc::create_i,
                                       host, static_cast<uint16_t>(port)),
                 NS_DISPATCH_NORMAL);
 
   // Wait until socket creation complete.
   mon.Wait();
@@ -1633,24 +1616,35 @@ void NrUdpSocketIpc::close_i() {
 
   if (socket_child_) {
     socket_child_->Close();
     socket_child_ = nullptr;
   }
 }
 
 #if defined(MOZILLA_INTERNAL_API)
+
+static void ReleaseIOThread_s()
+{
+  sThread->ReleaseUse();
+}
+
 // close(), but transfer the socket_child_ reference to die as well
 // static
-void NrUdpSocketIpc::release_child_i(nsIUDPSocketChild* aChild) {
+void NrUdpSocketIpc::destroy_i(nsIUDPSocketChild* aChild,
+                               nsCOMPtr<nsIEventTarget>& aStsThread) {
   RefPtr<nsIUDPSocketChild> socket_child_ref =
     already_AddRefed<nsIUDPSocketChild>(aChild);
   if (socket_child_ref) {
     socket_child_ref->Close();
   }
+
+  RUN_ON_THREAD(aStsThread,
+                WrapRunnableNM(&ReleaseIOThread_s),
+                NS_DISPATCH_NORMAL);
 }
 #endif
 
 void NrUdpSocketIpc::recv_callback_s(RefPtr<nr_udp_message> msg) {
   ASSERT_ON_THREAD(sts_thread_);
 
   {
     ReentrantMonitorAutoEnter mon(monitor_);
--- a/media/mtransport/nr_socket_prsock.h
+++ b/media/mtransport/nr_socket_prsock.h
@@ -275,17 +275,18 @@ private:
   nsresult SetAddress();  // Set the local address from parent info.
 
   // Main or private thread executors of the NrSocketBase APIs
   void create_i(const nsACString &host, const uint16_t port);
   void connect_i(const nsACString &host, const uint16_t port);
   void sendto_i(const net::NetAddr &addr, nsAutoPtr<DataBuffer> buf);
   void close_i();
 #if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API)
-  static void release_child_i(nsIUDPSocketChild* aChild);
+  static void destroy_i(nsIUDPSocketChild* aChild,
+                        nsCOMPtr<nsIEventTarget>& aStsThread);
 #endif
   // STS thread executor
   void recv_callback_s(RefPtr<nr_udp_message> msg);
 
   ReentrantMonitor monitor_; // protects err_and state_
   bool err_;
   NrSocketIpcState state_;