Bug 1554976 - Make mDNS service a singleton; r=mjf
☠☠ backed out by 5b1d527eacfc ☠ ☠
authorDan Minor <dminor@mozilla.com>
Wed, 28 Aug 2019 13:12:01 +0000
changeset 554154 3e16c10bb9660db569563792cbbb21083bf41391
parent 554153 6a404fca61dc42489fcae72a8d608526b9a2846a
child 554155 27b4dddf9597df076caad414b27f70ba8dbc7245
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmjf
bugs1554976, 1569311, 1569955
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1554976 - Make mDNS service a singleton; r=mjf The current code causes one mDNS service to be created for each PeerConnection. Due to Bug 1569311, the services persist until shutdown, which can lead to a lot of mDNS threads running on sites which use WebRTC for fingerprinting. This change makes it so we start at most one mDNS service. I've filed Bug 1569955 to look at shutting down the mDNS service after the last hostname is unregistered. As an alternative, if we fix Bug 1569311, we could also use refcounting and stop the mDNS service after the last StunAddrsRequestParent is freed. Differential Revision: https://phabricator.services.mozilla.com/D42151
media/mtransport/ipc/StunAddrsRequestParent.cpp
media/mtransport/ipc/StunAddrsRequestParent.h
--- a/media/mtransport/ipc/StunAddrsRequestParent.cpp
+++ b/media/mtransport/ipc/StunAddrsRequestParent.cpp
@@ -13,29 +13,39 @@
 
 #include "../mdns_service/mdns_service.h"
 
 using namespace mozilla::ipc;
 
 namespace mozilla {
 namespace net {
 
-StunAddrsRequestParent::StunAddrsRequestParent()
-    : mIPCClosed(false), mMDNSService(nullptr) {
+StunAddrsRequestParent::StunAddrsRequestParent() : mIPCClosed(false) {
   NS_GetMainThread(getter_AddRefs(mMainThread));
 
   nsresult res;
   mSTSThread = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &res);
   MOZ_ASSERT(mSTSThread);
+
+  // This means that the mDNS service will continue running until shutdown
+  // once started. The StunAddrsRequestParent destructor does not run until
+  // shutdown anyway (see Bug 1569311), so there is not much we can do about
+  // this here. One option would be to add a check if there are no hostnames
+  // registered after UnregisterHostname is called, and if so, stop the mDNS
+  // service at that time (see Bug 1569955.)
+  if (!mSharedMDNSService) {
+    mSharedMDNSService = new MDNSServiceWrapper;
+  }
 }
 
 StunAddrsRequestParent::~StunAddrsRequestParent() {
-  if (mMDNSService) {
-    mdns_service_stop(mMDNSService);
-    mMDNSService = nullptr;
+  ASSERT_ON_THREAD(mMainThread);
+
+  if (mSharedMDNSService) {
+    mSharedMDNSService = nullptr;
   }
 }
 
 mozilla::ipc::IPCResult StunAddrsRequestParent::RecvGetStunAddrs() {
   ASSERT_ON_THREAD(mMainThread);
 
   if (mIPCClosed) {
     return IPC_OK();
@@ -52,39 +62,31 @@ mozilla::ipc::IPCResult StunAddrsRequest
 mozilla::ipc::IPCResult StunAddrsRequestParent::RecvRegisterMDNSHostname(
     const nsCString& aHostname, const nsCString& aAddress) {
   ASSERT_ON_THREAD(mMainThread);
 
   if (mIPCClosed) {
     return IPC_OK();
   }
 
-  if (!mMDNSService) {
-    mMDNSService = mdns_service_start();
-  }
-
-  if (mMDNSService) {
-    mdns_service_register_hostname(mMDNSService, aHostname.BeginReading(),
-                                   aAddress.BeginReading());
-  }
+  mSharedMDNSService->RegisterHostname(aHostname.BeginReading(),
+                                       aAddress.BeginReading());
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult StunAddrsRequestParent::RecvUnregisterMDNSHostname(
     const nsCString& aHostname) {
   ASSERT_ON_THREAD(mMainThread);
 
   if (mIPCClosed) {
     return IPC_OK();
   }
 
-  if (mMDNSService) {
-    mdns_service_unregister_hostname(mMDNSService, aHostname.BeginReading());
-  }
+  mSharedMDNSService->UnregisterHostname(aHostname.BeginReading());
 
   return IPC_OK();
 }
 
 mozilla::ipc::IPCResult StunAddrsRequestParent::Recv__delete__() {
   // see note below in ActorDestroy
   mIPCClosed = true;
   return IPC_OK();
@@ -123,13 +125,48 @@ void StunAddrsRequestParent::SendStunAdd
     // nothing to do: child probably crashed
     return;
   }
 
   // send the new addresses back to the child
   Unused << SendOnStunAddrsAvailable(addrs);
 }
 
+StaticRefPtr<StunAddrsRequestParent::MDNSServiceWrapper>
+    StunAddrsRequestParent::mSharedMDNSService;
+
 NS_IMPL_ADDREF(StunAddrsRequestParent)
 NS_IMPL_RELEASE(StunAddrsRequestParent)
 
+void StunAddrsRequestParent::MDNSServiceWrapper::RegisterHostname(
+    const char* hostname, const char* address) {
+  StartIfRequired();
+  if (mMDNSService) {
+    mdns_service_register_hostname(mMDNSService, hostname, address);
+  }
+}
+
+void StunAddrsRequestParent::MDNSServiceWrapper::UnregisterHostname(
+    const char* hostname) {
+  StartIfRequired();
+  if (mMDNSService) {
+    mdns_service_unregister_hostname(mMDNSService, hostname);
+  }
+}
+
+StunAddrsRequestParent::MDNSServiceWrapper::~MDNSServiceWrapper() {
+  if (mMDNSService) {
+    mdns_service_stop(mMDNSService);
+    mMDNSService = nullptr;
+  }
+}
+
+void StunAddrsRequestParent::MDNSServiceWrapper::StartIfRequired() {
+  if (!mMDNSService) {
+    mMDNSService = mdns_service_start();
+  }
+}
+
+NS_IMPL_ADDREF(StunAddrsRequestParent::MDNSServiceWrapper)
+NS_IMPL_RELEASE(StunAddrsRequestParent::MDNSServiceWrapper)
+
 }  // namespace net
 }  // namespace mozilla
--- a/media/mtransport/ipc/StunAddrsRequestParent.h
+++ b/media/mtransport/ipc/StunAddrsRequestParent.h
@@ -43,15 +43,34 @@ class StunAddrsRequestParent : public PS
   void SendStunAddrs_m(const NrIceStunAddrArray& addrs);
 
   ThreadSafeAutoRefCnt mRefCnt;
   NS_DECL_OWNINGTHREAD
 
  private:
   bool mIPCClosed;  // true if IPDL channel has been closed (child crash)
 
-  MDNSService* mMDNSService;
+  class MDNSServiceWrapper {
+   public:
+    void RegisterHostname(const char* hostname, const char* address);
+    void UnregisterHostname(const char* hostname);
+
+    NS_IMETHOD_(MozExternalRefCountType) AddRef();
+    NS_IMETHOD_(MozExternalRefCountType) Release();
+
+   protected:
+    ThreadSafeAutoRefCnt mRefCnt;
+    NS_DECL_OWNINGTHREAD
+
+   private:
+    virtual ~MDNSServiceWrapper();
+    void StartIfRequired();
+
+    MDNSService* mMDNSService = nullptr;
+  };
+
+  static StaticRefPtr<MDNSServiceWrapper> mSharedMDNSService;
 };
 
 }  // namespace net
 }  // namespace mozilla
 
 #endif  // mozilla_net_StunAddrsRequestParent_h