From c3d8942d0f45ff123e3fe9d815ef176e919a4322 Mon Sep 17 00:00:00 2001
From: Dan Minor <dminor@mozilla.com>
Date: Wed, 28 Aug 2019 13:12:01 +0000
Subject: [PATCH] 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
---
 .../mtransport/ipc/StunAddrsRequestParent.cpp | 69 ++++++++++++++-----
 media/mtransport/ipc/StunAddrsRequestParent.h | 21 +++++-
 2 files changed, 73 insertions(+), 17 deletions(-)

diff --git a/media/mtransport/ipc/StunAddrsRequestParent.cpp b/media/mtransport/ipc/StunAddrsRequestParent.cpp
index 6c7d79c21851..fa15fdc26ae1 100644
--- a/media/mtransport/ipc/StunAddrsRequestParent.cpp
+++ b/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(),
-                                   aAddress.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
diff --git a/media/mtransport/ipc/StunAddrsRequestParent.h b/media/mtransport/ipc/StunAddrsRequestParent.h
index 057adf197fcc..9d64a00c3bdc 100644
--- a/media/mtransport/ipc/StunAddrsRequestParent.h
+++ b/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