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

--HG--
extra : moz-landing-system : lando

media/mtransport/ipc/StunAddrsRequestParent.cpp

@@ -18,19 +18,29 @@ 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;
   }
 }
 
@@ -57,14 +67,8 @@ mozilla::ipc::IPCResult StunAddrsRequestParent::RecvRegisterMDNSHostname(
     return IPC_OK();
   }
 
-  if (!mMDNSService) {
-    mMDNSService = mdns_service_start();
-  }
-
-  if (mMDNSService) {
-    mdns_service_register_hostname(mMDNSService, aHostname.BeginReading(),
+  mSharedMDNSService->RegisterHostname(aHostname.BeginReading(),
                                        aAddress.BeginReading());
-  }
 
   return IPC_OK();
 }
@@ -77,9 +81,7 @@ mozilla::ipc::IPCResult StunAddrsRequestParent::RecvUnregisterMDNSHostname(
     return IPC_OK();
   }
 
-  if (mMDNSService) {
-    mdns_service_unregister_hostname(mMDNSService, aHostname.BeginReading());
-  }
+  mSharedMDNSService->UnregisterHostname(aHostname.BeginReading());
 
   return IPC_OK();
 }
@@ -128,8 +130,43 @@ void StunAddrsRequestParent::SendStunAddrs_m(const NrIceStunAddrArray& addrs) {
   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

media/mtransport/ipc/StunAddrsRequestParent.h

@@ -48,7 +48,26 @@ class StunAddrsRequestParent : public PStunAddrsRequestParent {
  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