Improve our check for whether a thread is still starting up.
It turns out that there was a race in Thread::Create that could confuse our
suspend-count sanity checking. Tested by manually inserting a sleep(3) there.
I could have added an extra field to Thread, but since we only need to check
this once if we do a GC while a thread is still starting up, a simple string
comparison will be fine.
thread_list.cc:79] Thread[10,tid=7447,VmWait,Thread*=0x00d23060,peer=0x60f1b350,"<native thread without managed peer>"] suspend count already zero
I've also added some logging so we'll be nagged to remove a work-around for
a bionic bug when we're in a tree where the bionic bug is fixed.
Change-Id: I78ac3c58245c1ecff3e86dcf297b94ae0085f7a2
diff --git a/src/thread.cc b/src/thread.cc
index 4554cee..fe38c8e 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -72,6 +72,8 @@
static Method* gThreadGroup_removeThread = NULL;
static Method* gUncaughtExceptionHandler_uncaughtException = NULL;
+static const char* kThreadNameDuringStartup = "<native thread without managed peer>";
+
void Thread::InitCardTable() {
card_table_ = Runtime::Current()->GetHeap()->GetCardTable()->GetBiasedBegin();
}
@@ -109,6 +111,7 @@
void Thread::InitAfterFork() {
InitTid();
+
#if defined(__BIONIC__)
// Work around a bionic bug.
struct bionic_pthread_internal_t {
@@ -118,7 +121,12 @@
pid_t kernel_id;
// et cetera. we just need 'kernel_id' so we can stop here.
};
- reinterpret_cast<bionic_pthread_internal_t*>(pthread_self())->kernel_id = tid_;
+ bionic_pthread_internal_t* self = reinterpret_cast<bionic_pthread_internal_t*>(pthread_self());
+ if (self->kernel_id == tid_) {
+ // TODO: if you see this logging, you can remove this code!
+ LOG(INFO) << "*** this tree doesn't have the bionic pthread kernel_id bug";
+ }
+ self->kernel_id = tid_;
#endif
}
@@ -910,11 +918,21 @@
pre_allocated_OutOfMemoryError_(NULL),
debug_invoke_req_(new DebugInvokeReq),
trace_stack_(new std::vector<TraceStackFrame>),
- name_(new std::string("<native thread without managed peer>")) {
+ name_(new std::string(kThreadNameDuringStartup)) {
CHECK_EQ((sizeof(Thread) % 4), 0U) << sizeof(Thread);
memset(&held_mutexes_[0], 0, sizeof(held_mutexes_));
}
+bool Thread::IsStillStarting() const {
+ // You might think you can check whether the state is kStarting, but for much of thread startup,
+ // the thread might also be in kVmWait.
+ // You might think you can check whether the peer is NULL, but the peer is actually created and
+ // assigned fairly early on, and needs to be.
+ // It turns out that the last thing to change is the thread name; that's a good proxy for "has
+ // this thread _ever_ entered kRunnable".
+ return (*name_ == kThreadNameDuringStartup);
+}
+
static void MonitorExitVisitor(const Object* object, void*) {
Object* entered_monitor = const_cast<Object*>(object);
LOG(WARNING) << "Calling MonitorExit on object " << object << " (" << PrettyTypeOf(object) << ")"
diff --git a/src/thread.h b/src/thread.h
index 8ff9dfb..91ea568 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -202,6 +202,8 @@
return suspend_count_;
}
+ bool IsStillStarting() const;
+
// Returns the current Method* and native PC (not dex PC) for this thread.
Method* GetCurrentMethod(uintptr_t* pc = NULL, Method*** sp = NULL) const;
diff --git a/src/thread_list.cc b/src/thread_list.cc
index 80ab2cf..45b7966 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -75,7 +75,7 @@
#endif
if (delta == -1 && thread->suspend_count_ <= 0) {
// This is expected if you attach a thread during a GC.
- if (thread->GetState() != kStarting && thread->GetPeer() != NULL) {
+ if (!thread->IsStillStarting()) {
LOG(FATAL) << *thread << " suspend count already zero";
}
return;