Bug 1396652: Ensure ordered destruction of channel entries at shutdown. r?mixedpuppy draft
authorKris Maglione <maglione.k@gmail.com>
Mon, 04 Sep 2017 14:31:26 -0700
changeset 658798 9598a628b43208cd7a26c68cb888847429ca4e93
parent 658797 8b92938b98d9a4830afe4e7e280bd00727c29e58
child 658808 fc41b07272a2aac88174456d88079fd29e432477
push id77872
push usermaglione.k@gmail.com
push dateMon, 04 Sep 2017 21:31:55 +0000
reviewersmixedpuppy
bugs1396652
milestone57.0a1
Bug 1396652: Ensure ordered destruction of channel entries at shutdown. r?mixedpuppy When channel registrations aren't explicitly unregistred from JS, they're instead unregistered when the entry object is cycle collected. When the entries are created close to shutdown, that can leave some uncertainty as to the order of destruction, and the WebRequestService might wind up being destroyed before all of the entries. In that case, the registrations are cleaned up when the hash entries hash table is being destroyed. While that isn't strictly a problem, the entries expect to still be present in the hash table when they're being destroyed, as a basic sanity check. This patch ensures that we always remove entries from the hash table before it's destroyed, so those invariants are maintained. MozReview-Commit-ID: 5jWpFeFyjJZ
toolkit/components/extensions/webrequest/WebRequestService.cpp
toolkit/components/extensions/webrequest/WebRequestService.h
--- a/toolkit/components/extensions/webrequest/WebRequestService.cpp
+++ b/toolkit/components/extensions/webrequest/WebRequestService.cpp
@@ -22,16 +22,20 @@ using namespace mozilla::dom;
 
 WebRequestService::WebRequestService()
   : mDataLock("WebRequest service data lock")
 {
 }
 
 WebRequestService::~WebRequestService()
 {
+  for (auto iter = mChannelEntries.Iter(); !iter.Done(); iter.Next()) {
+    auto& channel = iter.Data();
+    channel->DetachAll();
+  }
 }
 
 NS_IMPL_ISUPPORTS(WebRequestService, mozIWebRequestService)
 
 /* static */ WebRequestService&
 WebRequestService::GetSingleton()
 {
   static RefPtr<WebRequestService> instance;
@@ -127,17 +131,18 @@ WebRequestService::ChannelParent::Detach
 
   removeFrom(entry->mTabParents);
   if (entry->mTabParents.isEmpty()) {
     map.Remove(mChannelId);
   }
   mDetached = true;
 }
 
-WebRequestService::ChannelEntry::~ChannelEntry()
+void
+WebRequestService::ChannelEntry::DetachAll()
 {
   while (ChannelParent* parent = mTabParents.getFirst()) {
     parent->Detach();
   }
 }
 
 WebRequestService::Destructor::~Destructor()
 {
--- a/toolkit/components/extensions/webrequest/WebRequestService.h
+++ b/toolkit/components/extensions/webrequest/WebRequestService.h
@@ -83,17 +83,18 @@ private:
   private:
     ChannelParent* mChannelParent;
     bool mDestructCalled;
   };
 
   class ChannelEntry
   {
   public:
-    ~ChannelEntry();
+    void DetachAll();
+
     // Note: We can't keep a strong pointer to the channel here, since channels
     // are not cycle collected, and a reference to this object will be stored on
     // the channel in order to keep the entry alive.
     nsWeakPtr mChannel;
     LinkedList<ChannelParent> mTabParents;
   };
 
   nsClassHashtable<nsUint64HashKey, ChannelEntry> mChannelEntries;