Ensure ThreadPool constructor does not return until fully initialized.
Thread pool was being deleted too early during runtime shutdown,
this was causing a GC running during runtime shutdown to
occasionally reference a deleted thread pool.
Fixed an error where the thread pool was being deleted before the
threads were done attaching by making the thread pool constructor
block until all of the threads are attached.
Change-Id: I5f0884a74d78c4541ee0e582112857077f3f594f
diff --git a/src/barrier.cc b/src/barrier.cc
index 9651828..052536a 100644
--- a/src/barrier.cc
+++ b/src/barrier.cc
@@ -4,8 +4,8 @@
namespace art {
-Barrier::Barrier()
- : count_(0),
+Barrier::Barrier(int count)
+ : count_(count),
lock_("GC barrier lock"),
condition_("GC barrier condition", lock_) {
}
diff --git a/src/barrier.h b/src/barrier.h
index 342890b..86ce2fe 100644
--- a/src/barrier.h
+++ b/src/barrier.h
@@ -25,7 +25,7 @@
class Barrier {
public:
- Barrier();
+ Barrier(int count);
virtual ~Barrier();
// Pass through the barrier, decrements the count but does not block.
diff --git a/src/barrier_test.cc b/src/barrier_test.cc
index 7b31e29..284be57 100644
--- a/src/barrier_test.cc
+++ b/src/barrier_test.cc
@@ -67,7 +67,7 @@
TEST_F(BarrierTest, CheckWait) {
Thread* self = Thread::Current();
ThreadPool thread_pool(num_threads);
- Barrier barrier;
+ Barrier barrier(0);
AtomicInteger count1 = 0;
AtomicInteger count2 = 0;
AtomicInteger count3 = 0;
@@ -124,7 +124,7 @@
TEST_F(BarrierTest, CheckPass) {
Thread* self = Thread::Current();
ThreadPool thread_pool(num_threads);
- Barrier barrier;
+ Barrier barrier(0);
AtomicInteger count = 0;
const int32_t num_tasks = num_threads * 4;
const int32_t num_sub_tasks = 128;
diff --git a/src/gc/mark_sweep.cc b/src/gc/mark_sweep.cc
index 0976f94..d6c44db 100644
--- a/src/gc/mark_sweep.cc
+++ b/src/gc/mark_sweep.cc
@@ -85,7 +85,7 @@
classes_marked_(0), overhead_time_(0),
work_chunks_created_(0), work_chunks_deleted_(0),
reference_count_(0),
- gc_barrier_(new Barrier),
+ gc_barrier_(new Barrier(0)),
large_object_lock_("large object lock"),
mark_stack_expand_lock_("mark stack expand lock") {
DCHECK(mark_stack_ != NULL);
diff --git a/src/runtime.cc b/src/runtime.cc
index 6dc8435..c84f1f6 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -115,8 +115,6 @@
}
Runtime::~Runtime() {
- heap_->DeleteThreadPool();
-
Thread* self = Thread::Current();
{
MutexLock mu(self, *Locks::runtime_shutdown_lock_);
@@ -133,6 +131,7 @@
// Make sure to let the GC complete if it is running.
heap_->WaitForConcurrentGcToComplete(self);
+ heap_->DeleteThreadPool();
// Make sure our internal threads are dead before we start tearing down things they're using.
Dbg::StopJdwp();
diff --git a/src/thread_pool.cc b/src/thread_pool.cc
index 3ded42a..1686ea6 100644
--- a/src/thread_pool.cc
+++ b/src/thread_pool.cc
@@ -26,6 +26,7 @@
void ThreadPoolWorker::Run() {
Thread* self = Thread::Current();
Task* task = NULL;
+ thread_pool_->creation_barier_.Wait(self);
while ((task = thread_pool_->GetTask(self)) != NULL) {
task->Run(self);
task->Finalize();
@@ -57,11 +58,16 @@
completion_condition_("task completion condition", task_queue_lock_),
started_(false),
shutting_down_(false),
- waiting_count_(0) {
+ waiting_count_(0),
+ // Add one since the caller of constructor waits on the barrier too.
+ creation_barier_(num_threads + 1) {
+ Thread* self = Thread::Current();
while (GetThreadCount() < num_threads) {
const std::string name = StringPrintf("Thread pool worker %zu", GetThreadCount());
threads_.push_back(new ThreadPoolWorker(this, name, ThreadPoolWorker::kDefaultStackSize));
}
+ // Wait for all of the threads to attach.
+ creation_barier_.Wait(self);
}
ThreadPool::~ThreadPool() {
diff --git a/src/thread_pool.h b/src/thread_pool.h
index c8f6056..668dfe0 100644
--- a/src/thread_pool.h
+++ b/src/thread_pool.h
@@ -20,6 +20,7 @@
#include <deque>
#include <vector>
+#include "barrier.h"
#include "closure.h"
#include "locks.h"
#include "../src/mutex.h"
@@ -114,6 +115,7 @@
// Work balance detection.
uint64_t start_time_ GUARDED_BY(task_queue_lock_);
uint64_t total_wait_time_;
+ Barrier creation_barier_;
friend class ThreadPoolWorker;
friend class WorkStealingWorker;