Bug 1648453 - Use CallbackObject to trigger cleanup while setting up the incumbent global r=smaug
☠☠ backed out by 5b891bc9d106 ☠ ☠
authorJon Coppeard <jcoppeard@mozilla.com>
Thu, 16 Jul 2020 12:54:55 +0000
changeset 540757 0afcee198ecd3feea00496becacbb575972cd157
parent 540756 623252539387bff544d97309104c7006d00c99f7
child 540758 473e7d55a25ef12ae8e5b70642b36a73f28bf459
push id121895
push userjcoppeard@mozilla.com
push dateThu, 16 Jul 2020 15:54:23 +0000
treeherderautoland@9598a75cca47 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1648453
milestone80.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 1648453 - Use CallbackObject to trigger cleanup while setting up the incumbent global r=smaug This also factors out FinalizationRegistry support into a separate class. The JS engine now passes a callback function and the incumbent global which are recorded in QueueCallback. FinalizationRegistryCleanup::DoCleanup creates a CallbackObject can calls it immediately (I originally tried creating it in QueueCallback but there were problems because this is called during GC). I coped most of this from the way promise reaction jobs work. I added FinalizationRegistryCleanupCallback; I don't know if that's overkill. Differential Revision: https://phabricator.services.mozilla.com/D83614
dom/webidl/FinalizationRegistry.webidl
dom/webidl/moz.build
xpcom/base/CycleCollectedJSContext.cpp
xpcom/base/CycleCollectedJSContext.h
new file mode 100644
--- /dev/null
+++ b/dom/webidl/FinalizationRegistry.webidl
@@ -0,0 +1,10 @@
+/* -*- Mode: IDL; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this file,
+ * You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * This IDL file contains a callback used to integrate JS FinalizationRegistry
+ * objects with the browser.
+ */
+
+callback FinalizationRegistryCleanupCallback = void();
--- a/dom/webidl/moz.build
+++ b/dom/webidl/moz.build
@@ -528,16 +528,17 @@ WEBIDL_FILES = [
     'FileMode.webidl',
     'FileReader.webidl',
     'FileReaderSync.webidl',
     'FileSystem.webidl',
     'FileSystemDirectoryEntry.webidl',
     'FileSystemDirectoryReader.webidl',
     'FileSystemEntry.webidl',
     'FileSystemFileEntry.webidl',
+    'FinalizationRegistry.webidl',
     'FocusEvent.webidl',
     'FontFace.webidl',
     'FontFaceSet.webidl',
     'FontFaceSource.webidl',
     'FormData.webidl',
     'Function.webidl',
     'GainNode.webidl',
     'Gamepad.webidl',
--- a/xpcom/base/CycleCollectedJSContext.cpp
+++ b/xpcom/base/CycleCollectedJSContext.cpp
@@ -21,16 +21,17 @@
 #include "mozilla/MemoryReporting.h"
 #include "mozilla/Sprintf.h"
 #include "mozilla/Telemetry.h"
 #include "mozilla/TimelineConsumers.h"
 #include "mozilla/TimelineMarker.h"
 #include "mozilla/Unused.h"
 #include "mozilla/dom/DOMException.h"
 #include "mozilla/dom/DOMJSClass.h"
+#include "mozilla/dom/FinalizationRegistryBinding.h"
 #include "mozilla/dom/ProfileTimelineMarkerBinding.h"
 #include "mozilla/dom/PromiseBinding.h"
 #include "mozilla/dom/PromiseDebugging.h"
 #include "mozilla/dom/PromiseRejectionEvent.h"
 #include "mozilla/dom/PromiseRejectionEventBinding.h"
 #include "mozilla/dom/RootedDictionary.h"
 #include "mozilla/dom/ScriptSettings.h"
 #include "mozilla/dom/UserActivation.h"
@@ -55,33 +56,33 @@ namespace mozilla {
 
 CycleCollectedJSContext::CycleCollectedJSContext()
     : mRuntime(nullptr),
       mJSContext(nullptr),
       mDoingStableStates(false),
       mTargetedMicroTaskRecursionDepth(0),
       mMicroTaskLevel(0),
       mDebuggerRecursionDepth(0),
-      mMicroTaskRecursionDepth(0) {
+      mMicroTaskRecursionDepth(0),
+      mFinalizationRegistryCleanup(this) {
   MOZ_COUNT_CTOR(CycleCollectedJSContext);
 
   nsCOMPtr<nsIThread> thread = do_GetCurrentThread();
   mOwningThread = thread.forget().downcast<nsThread>().take();
   MOZ_RELEASE_ASSERT(mOwningThread);
 }
 
 CycleCollectedJSContext::~CycleCollectedJSContext() {
   MOZ_COUNT_DTOR(CycleCollectedJSContext);
   // If the allocation failed, here we are.
   if (!mJSContext) {
     return;
   }
 
   JS::SetHostCleanupFinalizationRegistryCallback(mJSContext, nullptr, nullptr);
-  mFinalizationRegistriesToCleanUp.reset();
 
   JS_SetContextPrivate(mJSContext, nullptr);
 
   mRuntime->SetContext(nullptr);
   mRuntime->Shutdown(mJSContext);
 
   // Last chance to process any events.
   CleanupIDBTransactions(mBaseRecursionDepth);
@@ -97,16 +98,18 @@ CycleCollectedJSContext::~CycleCollected
   MOZ_ASSERT(mPendingMicroTaskRunnables.empty());
 
   mUncaughtRejections.reset();
   mConsumedRejections.reset();
 
   mAboutToBeNotifiedRejectedPromises.Clear();
   mPendingUnhandledRejections.Clear();
 
+  mFinalizationRegistryCleanup.Destroy();
+
   JS_DestroyContext(mJSContext);
   mJSContext = nullptr;
 
   nsCycleCollector_forgetJSContext();
 
   mozilla::dom::DestroyScriptSettings();
 
   mOwningThread->SetScriptObserver(nullptr);
@@ -140,19 +143,17 @@ nsresult CycleCollectedJSContext::Initia
                                          PromiseRejectionTrackerCallback, this);
   mUncaughtRejections.init(mJSContext,
                            JS::GCVector<JSObject*, 0, js::SystemAllocPolicy>(
                                js::SystemAllocPolicy()));
   mConsumedRejections.init(mJSContext,
                            JS::GCVector<JSObject*, 0, js::SystemAllocPolicy>(
                                js::SystemAllocPolicy()));
 
-  mFinalizationRegistriesToCleanUp.init(mJSContext);
-  JS::SetHostCleanupFinalizationRegistryCallback(
-      mJSContext, CleanupFinalizationRegistryCallback, this);
+  mFinalizationRegistryCleanup.Init();
 
   // Cast to PerThreadAtomCache for dom::GetAtomCache(JSContext*).
   JS_SetContextPrivate(mJSContext, static_cast<PerThreadAtomCache*>(this));
 
   nsCycleCollector_registerJSContext(this);
 
   return NS_OK;
 }
@@ -743,61 +744,100 @@ nsresult CycleCollectedJSContext::Notify
     }
 
     JS::RootedObject promiseObj(mCx->RootingCx(), promise->PromiseObj());
     mCx->mPendingUnhandledRejections.Remove(JS::GetPromiseID(promiseObj));
   }
   return NS_OK;
 }
 
-class CleanupFinalizationRegistriesRunnable : public CancelableRunnable {
+class FinalizationRegistryCleanup::CleanupRunnable : public CancelableRunnable {
  public:
-  explicit CleanupFinalizationRegistriesRunnable(
-      CycleCollectedJSContext* aContext)
-      : CancelableRunnable("CleanupFinalizationRegistriesRunnable"),
-        mContext(aContext) {}
-  NS_DECL_NSIRUNNABLE
+  explicit CleanupRunnable(FinalizationRegistryCleanup* aCleanupWork)
+      : CancelableRunnable("CleanupRunnable"), mCleanupWork(aCleanupWork) {}
+
+  // MOZ_CAN_RUN_SCRIPT_BOUNDARY until Runnable::Run is MOZ_CAN_RUN_SCRIPT.  See
+  // bug 1535398.
+  MOZ_CAN_RUN_SCRIPT_BOUNDARY
+  NS_IMETHODIMP Run() {
+    mCleanupWork->DoCleanup();
+    return NS_OK;
+  }
+
  private:
-  CycleCollectedJSContext* mContext;
+  FinalizationRegistryCleanup* mCleanupWork;
 };
 
-NS_IMETHODIMP
-CleanupFinalizationRegistriesRunnable::Run() {
-  if (mContext->mFinalizationRegistriesToCleanUp.empty()) {
-    return NS_OK;
+FinalizationRegistryCleanup::FinalizationRegistryCleanup(
+    CycleCollectedJSContext* aContext)
+    : mContext(aContext) {}
+
+void FinalizationRegistryCleanup::Destroy() {
+  // This must happen before the CycleCollectedJSContext destructor calls
+  // JS_DestroyContext().
+  mCallbacks.reset();
+}
+
+void FinalizationRegistryCleanup::Init() {
+  JSContext* cx = mContext->Context();
+  mCallbacks.init(cx);
+  JS::SetHostCleanupFinalizationRegistryCallback(cx, QueueCallback, this);
+}
+
+/* static */
+void FinalizationRegistryCleanup::QueueCallback(JSFunction* aDoCleanup,
+                                                JSObject* aIncumbentGlobal,
+                                                void* aData) {
+  FinalizationRegistryCleanup* cleanup =
+      static_cast<FinalizationRegistryCleanup*>(aData);
+  cleanup->QueueCallback(aDoCleanup, aIncumbentGlobal);
+}
+
+void FinalizationRegistryCleanup::QueueCallback(JSFunction* aDoCleanup,
+                                                JSObject* aIncumbentGlobal) {
+  bool firstCallback = mCallbacks.empty();
+
+  MOZ_ALWAYS_TRUE(mCallbacks.append(Callback{aDoCleanup, aIncumbentGlobal}));
+
+  if (firstCallback) {
+    RefPtr<CleanupRunnable> cleanup = new CleanupRunnable(this);
+    NS_DispatchToCurrentThread(cleanup.forget());
+  }
+}
+
+void FinalizationRegistryCleanup::DoCleanup() {
+  if (mCallbacks.empty()) {
+    return;
   }
 
   JS::RootingContext* cx = mContext->RootingCx();
 
-  JS::Rooted<CycleCollectedJSContext::ObjectVector> registries(cx);
-  std::swap(registries.get(), mContext->mFinalizationRegistriesToCleanUp.get());
-
-  JS::Rooted<JSObject*> registry(cx);
-  for (const auto& r : registries) {
-    registry = r;
+  JS::Rooted<CallbackVector> callbacks(cx);
+  std::swap(callbacks.get(), mCallbacks.get());
 
-    AutoEntryScript aes(registry, "cleanupFinalizationRegistry");
-    mozilla::Unused << JS::CleanupQueuedFinalizationRegistry(aes.cx(),
-                                                             registry);
-  }
-
-  return NS_OK;
-}
+  for (const Callback& callback : callbacks) {
+    JSObject* functionObj = JS_GetFunctionObject(callback.mCallbackFunction);
+    JS::RootedObject globalObj(cx, JS::GetNonCCWObjectGlobal(functionObj));
 
-/* static */
-void CycleCollectedJSContext::CleanupFinalizationRegistryCallback(
-    JSObject* aRegistry, void* aData) {
-  CycleCollectedJSContext* ccjs = static_cast<CycleCollectedJSContext*>(aData);
-  ccjs->QueueFinalizationRegistryForCleanup(aRegistry);
-}
+    nsIGlobalObject* incumbentGlobal =
+        xpc::NativeGlobal(callback.mIncumbentGlobal);
+    if (!incumbentGlobal) {
+      continue;
+    }
 
-void CycleCollectedJSContext::QueueFinalizationRegistryForCleanup(
-    JSObject* aRegistry) {
-  bool firstRegistry = mFinalizationRegistriesToCleanUp.empty();
-  MOZ_ALWAYS_TRUE(mFinalizationRegistriesToCleanUp.append(aRegistry));
-  if (firstRegistry) {
-    RefPtr<CleanupFinalizationRegistriesRunnable> cleanup =
-        new CleanupFinalizationRegistriesRunnable(this);
-    NS_DispatchToCurrentThread(cleanup.forget());
+    RefPtr<FinalizationRegistryCleanupCallback> cleanupCallback(
+        new FinalizationRegistryCleanupCallback(functionObj, globalObj, nullptr,
+                                                incumbentGlobal));
+
+    nsIGlobalObject* global =
+        xpc::NativeGlobal(cleanupCallback->CallbackPreserveColor());
+    if (global) {
+      cleanupCallback->Call("FinalizationRegistryCleanup::DoCleanup");
+    }
   }
 }
 
+void FinalizationRegistryCleanup::Callback::trace(JSTracer* trc) {
+  JS::UnsafeTraceRoot(trc, &mCallbackFunction, "mCallbackFunction");
+  JS::UnsafeTraceRoot(trc, &mIncumbentGlobal, "mIncumbentGlobal");
+}
+
 }  // namespace mozilla
--- a/xpcom/base/CycleCollectedJSContext.h
+++ b/xpcom/base/CycleCollectedJSContext.h
@@ -27,16 +27,17 @@
 class nsCycleCollectionNoteRootCallback;
 class nsIRunnable;
 class nsThread;
 class nsWrapperCache;
 
 namespace mozilla {
 class AutoSlowOperation;
 
+class CycleCollectedJSContext;
 class CycleCollectedJSRuntime;
 
 namespace dom {
 class Exception;
 class WorkerJSContext;
 class WorkletJSContext;
 }  // namespace dom
 
@@ -80,16 +81,50 @@ class MicroTaskRunnable {
   NS_INLINE_DECL_REFCOUNTING(MicroTaskRunnable)
   MOZ_CAN_RUN_SCRIPT virtual void Run(AutoSlowOperation& aAso) = 0;
   virtual bool Suppressed() { return false; }
 
  protected:
   virtual ~MicroTaskRunnable() = default;
 };
 
+// Support for JS FinalizationRegistry objects, which allow a JS callback to be
+// registered that is called when objects die.
+//
+// We keep a vector of functions that call back into the JS engine along
+// with their associated incumbent globals, one per FinalizationRegistry object
+// that has pending cleanup work. These are run in their own task.
+class FinalizationRegistryCleanup {
+ public:
+  explicit FinalizationRegistryCleanup(CycleCollectedJSContext* aContext);
+  void Init();
+  void Destroy();
+  void QueueCallback(JSFunction* aDoCleanup, JSObject* aIncumbentGlobal);
+  MOZ_CAN_RUN_SCRIPT void DoCleanup();
+
+ private:
+  static void QueueCallback(JSFunction* aDoCleanup, JSObject* aIncumbentGlobal,
+                            void* aData);
+
+  class CleanupRunnable;
+
+  struct Callback {
+    JSFunction* mCallbackFunction;
+    JSObject* mIncumbentGlobal;
+    void trace(JSTracer* trc);
+  };
+
+  // This object is part of CycleCollectedJSContext, so it's safe to have a raw
+  // pointer to its containing context here.
+  CycleCollectedJSContext* mContext;
+
+  using CallbackVector = JS::GCVector<Callback, 0, InfallibleAllocPolicy>;
+  JS::PersistentRooted<CallbackVector> mCallbacks;
+};
+
 class CycleCollectedJSContext : dom::PerThreadAtomCache, private JS::JobQueue {
   friend class CycleCollectedJSRuntime;
 
  protected:
   CycleCollectedJSContext();
   virtual ~CycleCollectedJSContext();
 
   MOZ_IS_CLASS_INIT
@@ -257,20 +292,16 @@ class CycleCollectedJSContext : dom::Per
   // headers.  The caller presumably knows this can run script (like everything
   // in SpiderMonkey!) and will deal.
   MOZ_CAN_RUN_SCRIPT_BOUNDARY
   void runJobs(JSContext* cx) override;
   bool empty() const override;
   class SavedMicroTaskQueue;
   js::UniquePtr<SavedJobQueue> saveJobQueue(JSContext*) override;
 
-  static void CleanupFinalizationRegistryCallback(JSObject* aRegistry,
-                                                  void* aData);
-  void QueueFinalizationRegistryForCleanup(JSObject* aRegistry);
-
  private:
   CycleCollectedJSRuntime* mRuntime;
 
   JSContext* mJSContext;
 
   nsCOMPtr<dom::Exception> mPendingException;
   nsThread* mOwningThread;  // Manual refcounting to avoid include hell.
 
@@ -335,24 +366,17 @@ class CycleCollectedJSContext : dom::Per
 
     nsresult Cancel() final;
 
    private:
     CycleCollectedJSContext* mCx;
     PromiseArray mUnhandledRejections;
   };
 
-  // Support for JS FinalizationRegistry objects.
-  //
-  // These allow a JS callback to be registered that is called when an object
-  // dies. The browser part of the implementation keeps a vector of
-  // FinalizationRegistries with pending callbacks here.
-  friend class CleanupFinalizationRegistriesRunnable;
-  using ObjectVector = JS::GCVector<JSObject*, 0, InfallibleAllocPolicy>;
-  JS::PersistentRooted<ObjectVector> mFinalizationRegistriesToCleanUp;
+  FinalizationRegistryCleanup mFinalizationRegistryCleanup;
 };
 
 class MOZ_STACK_CLASS nsAutoMicroTask {
  public:
   nsAutoMicroTask() {
     CycleCollectedJSContext* ccjs = CycleCollectedJSContext::Get();
     if (ccjs) {
       ccjs->EnterMicroTask();