Bug 1628201: HelperThreadTaskHandler should use UniquePtr for mOffThreadTask. r=KrisWright
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Thu, 16 Apr 2020 10:57:02 +0000
changeset 524571 83bb64e5b26d6a318400fad084afdc7d26afd68b
parent 524570 6eb1cc169dbf3f258bed8cb747ced0ebfb15188d
child 524572 dd0e24feda2d5e7825915c7d8df947b4cde4b792
push id37323
push userdluca@mozilla.com
push dateFri, 17 Apr 2020 16:25:55 +0000
treeherdermozilla-central@b4b1d6f91ef0 [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
@@ -1904,17 +1904,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)(js::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,19 @@ 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.
  */