Bug 1628201: HelperThreadTaskHandler should use UniquePtr for mOffThreadTask. r=KrisWright
☠☠ backed out by b760586ab7e6 ☠ ☠
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Wed, 08 Apr 2020 19:38:32 +0000
changeset 523223 152d46945b27874ddb2329e4cddeaa8edfe95c69
parent 523222 eb7723f5bdd5a668b86018a6e2133f776bfde244
child 523224 663509b17a839f0feca8df100e5a8f73448f2632
push id112512
push userallstars.chh@gmail.com
push dateThu, 09 Apr 2020 13:02:16 +0000
treeherderautoland@152d46945b27 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersKrisWright
bugs1628201
milestone77.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 1628201: HelperThreadTaskHandler should use UniquePtr for mOffThreadTask. r=KrisWright Differential Revision: https://phabricator.services.mozilla.com/D70155
js/public/Utility.h
js/src/jsapi.cpp
js/src/jsapi.h
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
js/xpconnect/src/XPCJSRuntime.cpp
js/xpconnect/src/XPCJSThreadPool.h
--- a/js/public/Utility.h
+++ b/js/public/Utility.h
@@ -70,16 +70,17 @@ enum ThreadType {
 /*
  * Threads need a universal way to dispatch from xpcom thread pools,
  * so having objects inherit from this struct enables
  * mozilla::HelperThreadPool's runnable handler to call runTask() on each type.
  */
 struct RunnableTask {
   virtual ThreadType threadType() = 0;
   virtual void runTask() = 0;
+  virtual ~RunnableTask() = default;
 };
 
 namespace oom {
 
 /*
  * Theads are tagged only in certain debug contexts.  Notably, to make testing
  * OOM in certain helper threads more effective, we allow restricting the OOM
  * testing to a certain helper thread type. This allows us to fail e.g. in
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -1903,17 +1903,17 @@ JS_PUBLIC_API JSObject* JS_NewObjectForC
 JS_PUBLIC_API bool JS_IsNative(JSObject* obj) { return obj->isNative(); }
 
 JS_PUBLIC_API void JS::AssertObjectBelongsToCurrentThread(JSObject* obj) {
   JSRuntime* rt = obj->compartment()->runtimeFromAnyThread();
   MOZ_RELEASE_ASSERT(CurrentThreadCanAccessRuntime(rt));
 }
 
 JS_PUBLIC_API void SetHelperThreadTaskCallback(
-    void (*callback)(js::RunnableTask*)) {
+    void (*callback)(js::UniquePtr<js::RunnableTask>)) {
   HelperThreadTaskCallback = callback;
 }
 
 JS_PUBLIC_API void JS::SetFilenameValidationCallback(
     JS::FilenameValidationCallback cb) {
   js::gFilenameValidationCallback = cb;
 }
 
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -306,17 +306,17 @@ using FilenameValidationCallback = bool 
 JS_PUBLIC_API void SetFilenameValidationCallback(FilenameValidationCallback cb);
 
 } /* namespace JS */
 
 /**
  * Set callback to send tasks to XPCOM thread pools
  */
 JS_PUBLIC_API void SetHelperThreadTaskCallback(
-    void (*callback)(js::RunnableTask*));
+    void (*callback)(js::UniquePtr<js::RunnableTask>));
 
 extern JS_PUBLIC_API const char* JS_GetImplementationVersion(void);
 
 extern JS_PUBLIC_API void JS_SetDestroyCompartmentCallback(
     JSContext* cx, JSDestroyCompartmentCallback callback);
 
 extern JS_PUBLIC_API void JS_SetSizeOfIncludingThisCompartmentCallback(
     JSContext* cx, JSSizeOfIncludingThisCompartmentCallback callback);
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -66,17 +66,17 @@ using mozilla::PositiveInfinity;
 /* static */ MOZ_THREAD_LOCAL(JSContext*) js::TlsContext;
 /* static */
 Atomic<size_t> JSRuntime::liveRuntimesCount;
 Atomic<JS::LargeAllocationFailureCallback> js::OnLargeAllocationFailure;
 
 JS::FilenameValidationCallback js::gFilenameValidationCallback = nullptr;
 
 namespace js {
-void (*HelperThreadTaskCallback)(js::RunnableTask*);
+void (*HelperThreadTaskCallback)(UniquePtr<RunnableTask>);
 
 bool gCanUseExtraThreads = true;
 }  // namespace js
 
 void js::DisableExtraThreads() { gCanUseExtraThreads = false; }
 
 const JSSecurityCallbacks js::NullSecurityCallbacks = {};
 
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1111,13 +1111,13 @@ extern mozilla::Atomic<JS::LargeAllocati
 // This callback is set by JS::SetBuildIdOp and may be null. See comment in
 // jsapi.h.
 extern mozilla::Atomic<JS::BuildIdOp> GetBuildId;
 
 extern JS::FilenameValidationCallback gFilenameValidationCallback;
 
 // This callback is set by js::SetHelperThreadTaskCallback and may be null.
 // See comment in jsapi.h.
-extern void (*HelperThreadTaskCallback)(js::RunnableTask*);
+extern void (*HelperThreadTaskCallback)(js::UniquePtr<js::RunnableTask>);
 
 } /* namespace js */
 
 #endif /* vm_Runtime_h */
--- a/js/xpconnect/src/XPCJSRuntime.cpp
+++ b/js/xpconnect/src/XPCJSRuntime.cpp
@@ -1055,18 +1055,18 @@ void XPCJSRuntime::SystemIsBeingShutDown
   // making them !IsValid anyway through SystemIsBeingShutDown.
   mWrappedJSRoots = nullptr;
 }
 
 StaticAutoPtr<HelperThreadPool> gHelperThreads;
 
 void InitializeHelperThreadPool() { gHelperThreads = new HelperThreadPool(); }
 
-void DispatchOffThreadTask(RunnableTask* task) {
-  gHelperThreads->Dispatch(MakeAndAddRef<HelperThreadTaskHandler>(task));
+void DispatchOffThreadTask(js::UniquePtr<RunnableTask> task) {
+  gHelperThreads->Dispatch(MakeAndAddRef<HelperThreadTaskHandler>(std::move(task)));
 }
 
 void XPCJSRuntime::Shutdown(JSContext* cx) {
   // This destructor runs before ~CycleCollectedJSContext, which does the actual
   // JS_DestroyContext() call. But destroying the context triggers one final GC,
   // which can call back into the context with various callbacks if we aren't
   // careful. Remove the relevant callbacks, but leave the weak pointer
   // callbacks to clear out any remaining table entries.
--- a/js/xpconnect/src/XPCJSThreadPool.h
+++ b/js/xpconnect/src/XPCJSThreadPool.h
@@ -2,38 +2,40 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* 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/. */
 
 #ifndef mozilla_JSThreadPool_h
 #define mozilla_JSThreadPool_h
 
+#include "mozilla/Unused.h"
 #include "nsIThreadPool.h"
 #include "js/Utility.h"
+#include "js/UniquePtr.h"
 
 namespace mozilla {
 /*
  * Since XPCOM thread pools dispatch nsIRunnable*, we want a way to take the
  * specific job that JS wants to dispatch offthread, and run it via an
  * nsIRunnable(). RunnableTasks should self-destroy.
  */
 class HelperThreadTaskHandler : public Runnable {
  public:
   NS_IMETHOD Run() override {
     mOffThreadTask->runTask();
-    mOffThreadTask = nullptr;
+    Unused << mOffThreadTask.release();
     return NS_OK;
   }
-  explicit HelperThreadTaskHandler(RunnableTask* task)
-      : Runnable("HelperThreadTaskHandler"), mOffThreadTask(task) {}
+  explicit HelperThreadTaskHandler(js::UniquePtr<RunnableTask> task)
+      : Runnable("HelperThreadTaskHandler"), mOffThreadTask(std::move(task)) {}
 
  private:
   ~HelperThreadTaskHandler() = default;
-  RunnableTask* mOffThreadTask;
+  js::UniquePtr<RunnableTask> mOffThreadTask;
 };
 
 /*
  * HelperThreadPool is a thread pool infrastructure for JS offthread tasks. It
  * holds the actual instance of a thread pool that we instantiate. The intention
  * is to have a way to manage some of the things GlobalHelperThreadState does
  * independently of nsThreadPool.
  */