Bug 1628201: HelperThreadTaskHandler should use UniquePtr for mOffThreadTask. r=KrisWright
☠☠ backed out by 8e0175d27817 ☠ ☠
authorYoshi Cheng-Hao Huang <allstars.chh@gmail.com>
Fri, 10 Apr 2020 12:52:35 +0000
changeset 523405 ad3f58c996e59bc5ca0a0e6aac61c8f6a7e51096
parent 523404 40150e24e05f9ae3736db83b9b380047404650ad
child 523406 6392325ec39e5cb10edd4aaaa4afb7a9f061b0b2
push id112634
push userallstars.chh@gmail.com
push dateFri, 10 Apr 2020 13:00:04 +0000
treeherderautoland@ad3f58c996e5 [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)(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.
  */