Bug 1308655 - Remove js::Thread's infallible constructor. r=froydnj
authorJan de Mooij <jdemooij@mozilla.com>
Wed, 12 Oct 2016 11:38:14 +0200
changeset 317504 8efe890d2a28a926df3f3fe0f40c291d84f61573
parent 317503 477890efdb8817ea5aaa54ef3cb17a2ba335bdb5
child 317505 b987bb9ad1a19f04b7223cca8aa098eb5f7782ed
push id82681
push userjandemooij@gmail.com
push dateWed, 12 Oct 2016 09:38:40 +0000
treeherdermozilla-inbound@b987bb9ad1a1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersfroydnj
bugs1308655
milestone52.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 1308655 - Remove js::Thread's infallible constructor. r=froydnj
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
@@ -3528,17 +3528,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)