From 950a08f68d9b5c152a78bea2f0daeda9f92ae69c Mon Sep 17 00:00:00 2001
From: Jon Coppeard <jcoppeard@mozilla.com>
Date: Fri, 17 Sep 2021 06:33:09 +0000
Subject: [PATCH] Bug 1730534 - Part 4: Assert that there's only one
 JSHolderMap::Iter at any time r=mccr8

This iterator can update the map for removed items so it's not safe to have
more than one live at any one time.

Differential Revision: https://phabricator.services.mozilla.com/D125431
---
 xpcom/base/CycleCollectedJSRuntime.cpp |  5 +++++
 xpcom/base/CycleCollectedJSRuntime.h   | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/xpcom/base/CycleCollectedJSRuntime.cpp b/xpcom/base/CycleCollectedJSRuntime.cpp
index 5b1c5744851e..7949ead695a7 100644
--- a/xpcom/base/CycleCollectedJSRuntime.cpp
+++ b/xpcom/base/CycleCollectedJSRuntime.cpp
@@ -500,6 +500,11 @@ void JSHolderMap::EntryVectorIter::Settle() {
 
 inline JSHolderMap::Iter::Iter(JSHolderMap& aMap, WhichHolders aWhich)
     : mHolderMap(aMap), mIter(aMap, aMap.mAnyZoneJSHolders) {
+#ifdef DEBUG
+  MOZ_RELEASE_ASSERT(!mHolderMap.mHasIterator);
+  mHolderMap.mHasIterator = true;
+#endif
+
   // Populate vector of zones to iterate after the any-zone holders.
   for (auto i = aMap.mPerZoneJSHolders.iter(); !i.done(); i.next()) {
     JS::Zone* zone = i.get().key();
diff --git a/xpcom/base/CycleCollectedJSRuntime.h b/xpcom/base/CycleCollectedJSRuntime.h
index 585b95e52eda..1ff15ba9219d 100644
--- a/xpcom/base/CycleCollectedJSRuntime.h
+++ b/xpcom/base/CycleCollectedJSRuntime.h
@@ -94,6 +94,10 @@ class JSHolderMap {
 
   JSHolderMap();
 
+#ifdef DEBUG
+  ~JSHolderMap() { MOZ_RELEASE_ASSERT(!mHasIterator); }
+#endif
+
   bool Has(void* aHolder) const;
   nsScriptObjectTracer* Get(void* aHolder) const;
   nsScriptObjectTracer* Extract(void* aHolder);
@@ -139,6 +143,12 @@ class JSHolderMap {
   // Currently this will only contain wrapper cache wrappers since these are the
   // only holders to pass a zone parameter through to AddJSHolder.
   EntryVectorMap mPerZoneJSHolders;
+
+#ifdef DEBUG
+  // Iterators can mutate the element vectors by removing stale elements. Allow
+  // at most one to exist at a time.
+  bool mHasIterator = false;
+#endif
 };
 
 // An iterator over an EntryVector that skips over removed entries and removes
@@ -174,6 +184,13 @@ class JSHolderMap::Iter {
  public:
   explicit Iter(JSHolderMap& aMap, WhichHolders aWhich = AllHolders);
 
+#ifdef DEBUG
+  ~Iter() {
+    MOZ_RELEASE_ASSERT(mHolderMap.mHasIterator);
+    mHolderMap.mHasIterator = false;
+  }
+#endif
+
   bool Done() const { return mIter.Done(); }
   const Entry& Get() const { return mIter.Get(); }
   void Next() {