Bug 1140773: Ensure that GCParallelTask subclasses properly join at the start of their destructor. r=shu
authorJim Blandy <jimb@mozilla.com>
Sat, 21 Mar 2015 22:18:50 -0700
changeset 265264 b40f0e7c51a08fac107491abc0fee29015cf7965
parent 265263 b7eb9be5ed3f46be26fb2b0ddb677a0a10185cbc
child 265265 45366e6959e2949aa30e7357960662c8287c300a
push id830
push userraliiev@mozilla.com
push dateFri, 19 Jun 2015 19:24:37 +0000
treeherdermozilla-release@932614382a68 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersshu
bugs1140773
milestone39.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 1140773: Ensure that GCParallelTask subclasses properly join at the start of their destructor. r=shu
js/src/gc/Nursery.cpp
js/src/jsgc.cpp
js/src/jsgc.h
js/src/vm/HelperThreads.cpp
--- a/js/src/gc/Nursery.cpp
+++ b/js/src/gc/Nursery.cpp
@@ -37,16 +37,17 @@ using mozilla::ArrayLength;
 using mozilla::PodCopy;
 using mozilla::PodZero;
 
 struct js::Nursery::FreeHugeSlotsTask : public GCParallelTask
 {
     explicit FreeHugeSlotsTask(FreeOp *fop) : fop_(fop) {}
     bool init() { return slots_.init(); }
     void transferSlotsToFree(HugeSlotsSet &slotsToFree);
+    ~FreeHugeSlotsTask() { join(); }
 
   private:
     FreeOp *fop_;
     HugeSlotsSet slots_;
 
     virtual void run() override;
 };
 
--- a/js/src/jsgc.cpp
+++ b/js/src/jsgc.cpp
@@ -2398,16 +2398,17 @@ struct UpdateCellPointersTask : public G
 #ifdef DEBUG
     static const unsigned ArenasToProcess = 16;
 #else
     static const unsigned ArenasToProcess = 256;
 #endif
 
     UpdateCellPointersTask() : rt_(nullptr), source_(nullptr), arenaList_(nullptr) {}
     void init(JSRuntime *rt, ArenasToUpdate *source, AutoLockHelperThreadState& lock);
+    ~UpdateCellPointersTask() { join(); }
 
   private:
     JSRuntime *rt_;
     ArenasToUpdate *source_;
     ArenaHeader *arenaList_;
 
     virtual void run() override;
     void getArenasToUpdate(AutoLockHelperThreadState& lock);
--- a/js/src/jsgc.h
+++ b/js/src/jsgc.h
@@ -987,16 +987,19 @@ class GCParallelTask
   protected:
     // A flag to signal a request for early completion of the off-thread task.
     mozilla::Atomic<bool> cancel_;
 
     virtual void run() = 0;
 
   public:
     GCParallelTask() : state(NotStarted), duration_(0) {}
+
+    // Derived classes must override this to ensure that join() gets called
+    // before members get destructed.
     virtual ~GCParallelTask();
 
     // Time spent in the most recent invocation of this task.
     int64_t duration() const { return duration_; }
 
     // The simple interface to a parallel task works exactly like pthreads.
     bool start();
     void join();
--- a/js/src/vm/HelperThreads.cpp
+++ b/js/src/vm/HelperThreads.cpp
@@ -737,17 +737,22 @@ GlobalHelperThreadState::canStartGCHelpe
 bool
 GlobalHelperThreadState::canStartGCParallelTask()
 {
     return !gcParallelWorklist().empty();
 }
 
 js::GCParallelTask::~GCParallelTask()
 {
-    join();
+    // Only most-derived classes' destructors may do the join: base class
+    // destructors run after those for derived classes' members, so a join in a
+    // base class can't ensure that the task is done using the members. All we
+    // can do now is check that someone has previously stopped the task.
+    AutoLockHelperThreadState helperLock;
+    MOZ_ASSERT(state == NotStarted);
 }
 
 bool
 js::GCParallelTask::startWithLockHeld()
 {
     MOZ_ASSERT(HelperThreadState().isLocked());
 
     // Tasks cannot be started twice.