Bug 1308655 - Remove js::Thread's infallible constructor. r=froydnj, a=gchang
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 12 Oct 2016 11:38:14 +0200
changeset 356415 179f83a4ec89c16d0c764429c5f3b84fba407e9b
parent 356414 9bfa86a9d4d5e56ae78ef422a9bedd50f0de5caf
child 356416 8b531cab21704362094a70e8c8d552ff3d00cf2b
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj, gchang
bugs1308655
milestone51.0a2
Bug 1308655 - Remove js::Thread's infallible constructor. r=froydnj, a=gchang
js/src/jsapi-tests/testSharedImmutableStringsCache.cpp
js/src/jsapi-tests/testThreadingExclusiveData.cpp
js/src/jsapi-tests/testThreadingThread.cpp
js/src/shell/js.cpp
js/src/threading/Thread.h
--- a/js/src/jsapi-tests/testSharedImmutableStringsCache.cpp
+++ b/js/src/jsapi-tests/testSharedImmutableStringsCache.cpp
@@ -63,17 +63,18 @@ BEGIN_TEST(testSharedImmutableStringsCac
     auto& cache = *maybeCache;
 
     js::Vector<js::Thread> threads(cx);
     CHECK(threads.reserve(NUM_THREADS));
 
     for (auto i : mozilla::MakeRange(NUM_THREADS)) {
         auto cacheAndIndex = js_new<CacheAndIndex>(&cache, i);
         CHECK(cacheAndIndex);
-        threads.infallibleEmplaceBack(getString, cacheAndIndex);
+        threads.infallibleEmplaceBack();
+        CHECK(threads.back().init(getString, cacheAndIndex));
     }
 
     for (auto& thread : threads)
         thread.join();
 
     return true;
 }
 END_TEST(testSharedImmutableStringsCache)
--- a/js/src/jsapi-tests/testThreadingExclusiveData.cpp
+++ b/js/src/jsapi-tests/testThreadingExclusiveData.cpp
@@ -72,17 +72,18 @@ BEGIN_TEST(testExclusiveData)
     js::ExclusiveData<uint64_t> counter(0);
 
     js::Vector<js::Thread> threads(cx);
     CHECK(threads.reserve(NumThreads));
 
     for (auto i : mozilla::MakeRange(NumThreads)) {
         auto counterAndBit = js_new<CounterAndBit>(i, counter);
         CHECK(counterAndBit);
-        CHECK(threads.emplaceBack(setBitAndCheck, counterAndBit));
+        CHECK(threads.emplaceBack());
+        CHECK(threads.back().init(setBitAndCheck, counterAndBit));
     }
 
     for (auto& thread : threads)
         thread.join();
 
     return true;
 }
 END_TEST(testExclusiveData)
--- a/js/src/jsapi-tests/testThreadingThread.cpp
+++ b/js/src/jsapi-tests/testThreadingThread.cpp
@@ -13,66 +13,71 @@
 #include "jsalloc.h"
 
 #include "jsapi-tests/tests.h"
 #include "threading/Thread.h"
 
 BEGIN_TEST(testThreadingThreadJoin)
 {
     bool flag = false;
-    js::Thread thread([](bool* flagp){*flagp = true;}, &flag);
+    js::Thread thread;
+    CHECK(thread.init([](bool* flagp){*flagp = true;}, &flag));
     CHECK(thread.joinable());
     thread.join();
     CHECK(flag);
     CHECK(!thread.joinable());
     return true;
 }
 END_TEST(testThreadingThreadJoin)
 
 BEGIN_TEST(testThreadingThreadDetach)
 {
     // We are going to detach this thread. Unlike join, we can't have it pointing at the stack
     // because it might do the write after we have returned and pushed a new frame.
     bool* flag = js_new<bool>(false);
-    js::Thread thread([](bool* flag){*flag = true; js_delete(flag);}, mozilla::Move(flag));
+    js::Thread thread;
+    CHECK(thread.init([](bool* flag){*flag = true; js_delete(flag);}, mozilla::Move(flag)));
     CHECK(thread.joinable());
     thread.detach();
     CHECK(!thread.joinable());
 
     return true;
 }
 END_TEST(testThreadingThreadDetach)
 
 BEGIN_TEST(testThreadingThreadSetName)
 {
-    js::Thread thread([](){js::ThisThread::SetName("JSAPI Test Thread");});
+    js::Thread thread;
+    CHECK(thread.init([](){js::ThisThread::SetName("JSAPI Test Thread");}));
     thread.detach();
     return true;
 }
 END_TEST(testThreadingThreadSetName)
 
 BEGIN_TEST(testThreadingThreadId)
 {
     CHECK(js::Thread::Id() == js::Thread::Id());
     js::Thread::Id fromOther;
-    js::Thread thread([](js::Thread::Id* idp){*idp = js::ThisThread::GetId();}, &fromOther);
+    js::Thread thread;
+    CHECK(thread.init([](js::Thread::Id* idp){*idp = js::ThisThread::GetId();}, &fromOther));
     js::Thread::Id fromMain = thread.get_id();
     thread.join();
     CHECK(fromOther == fromMain);
     return true;
 }
 END_TEST(testThreadingThreadId)
 
 BEGIN_TEST(testThreadingThreadVectorMoveConstruct)
 {
     const static size_t N = 10;
     mozilla::Atomic<int> count(0);
     mozilla::Vector<js::Thread, 0, js::SystemAllocPolicy> v;
     for (auto i : mozilla::MakeRange(N)) {
-        CHECK(v.emplaceBack([](mozilla::Atomic<int>* countp){(*countp)++;}, &count));
+        CHECK(v.emplaceBack());
+        CHECK(v.back().init([](mozilla::Atomic<int>* countp){(*countp)++;}, &count));
         CHECK(v.length() == i + 1);
     }
     for (auto& th : v)
         th.join();
     CHECK(count == 10);
     return true;
 }
 END_TEST(testThreadingThreadVectorMoveConstruct)
@@ -83,15 +88,16 @@ END_TEST(testThreadingThreadVectorMoveCo
 // trampoline, causing us to read through the reference when passing |bool bb|
 // from the trampoline. If the parent runs before the child, the bool may have
 // already become false, causing the trampoline to read the changed value, thus
 // causing the child's assertion to fail.
 BEGIN_TEST(testThreadingThreadArgCopy)
 {
     for (size_t i = 0; i < 10000; ++i) {
         bool b = true;
-        js::Thread thread([](bool bb){MOZ_RELEASE_ASSERT(bb);}, b);
+        js::Thread thread;
+        CHECK(thread.init([](bool bb){MOZ_RELEASE_ASSERT(bb);}, b));
         b = false;
         thread.join();
     }
     return true;
 }
 END_TEST(testThreadingThreadArgCopy)
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -3482,17 +3482,20 @@ ScheduleWatchdog(JSContext* cx, double t
         return true;
     }
 
     auto interval = TimeDuration::FromSeconds(t);
     auto timeout = TimeStamp::Now() + interval;
     LockGuard<Mutex> guard(sc->watchdogLock);
     if (!sc->watchdogThread) {
         MOZ_ASSERT(!sc->watchdogTimeout);
-        sc->watchdogThread.emplace(WatchdogMain, cx);
+        sc->watchdogThread.emplace();
+        AutoEnterOOMUnsafeRegion oomUnsafe;
+        if (!sc->watchdogThread->init(WatchdogMain, cx))
+            oomUnsafe.crash("watchdogThread.init");
     } else if (!sc->watchdogTimeout || timeout < sc->watchdogTimeout.value()) {
         sc->watchdogWakeup.notify_one();
     }
     sc->watchdogTimeout = Some(timeout);
     return true;
 }
 
 static void
--- a/js/src/threading/Thread.h
+++ b/js/src/threading/Thread.h
@@ -94,30 +94,22 @@ public:
             typename DerefO = typename mozilla::RemoveReference<NonConstO>::Type,
             typename = typename mozilla::EnableIf<mozilla::IsSame<DerefO, Options>::value,
                                                   void*>::Type>
   explicit Thread(O&& options = Options())
     : id_(Id())
     , options_(mozilla::Forward<O>(options))
   { }
 
-  // Start a thread of execution at functor |f| with parameters |args|. Note
-  // that the arguments must be either POD or rvalue references (mozilla::Move).
-  // Attempting to pass a reference will result in the value being copied, which
-  // may not be the intended behavior. See the comment below on
-  // ThreadTrampoline::args for an explanation.
-  template <typename F, typename... Args>
-  explicit Thread(F&& f, Args&&... args) {
-    MOZ_RELEASE_ASSERT(init(mozilla::Forward<F>(f),
-                            mozilla::Forward<Args>(args)...));
-  }
-
   // Start a thread of execution at functor |f| with parameters |args|. This
   // method will return false if thread creation fails. This Thread must not
-  // already have been created.
+  // already have been created. Note that the arguments must be either POD or
+  // rvalue references (mozilla::Move). Attempting to pass a reference will
+  // result in the value being copied, which may not be the intended behavior.
+  // See the comment below on ThreadTrampoline::args for an explanation.
   template <typename F, typename... Args>
   MOZ_MUST_USE bool init(F&& f, Args&&... args) {
     MOZ_RELEASE_ASSERT(!joinable());
     using Trampoline = detail::ThreadTrampoline<F, Args...>;
     AutoEnterOOMUnsafeRegion oom;
     auto trampoline = js_new<Trampoline>(mozilla::Forward<F>(f),
                                          mozilla::Forward<Args>(args)...);
     if (!trampoline)