Fix AttachCurrentThread to use the right thread group.

Change-Id: I9818845c005563d894a571edc4f9be9862312380
diff --git a/src/compiler.cc b/src/compiler.cc
index 72cabbf..b56fd49 100644
--- a/src/compiler.cc
+++ b/src/compiler.cc
@@ -921,7 +921,7 @@
     WorkerThread* worker = reinterpret_cast<WorkerThread*>(arg);
     Runtime* runtime = Runtime::Current();
     if (worker->spawn_) {
-      runtime->AttachCurrentThread("Compiler Worker", true);
+      runtime->AttachCurrentThread("Compiler Worker", true, NULL);
     }
     Thread::Current()->SetState(Thread::kRunnable);
     worker->Run();
diff --git a/src/debugger.cc b/src/debugger.cc
index 1f7b0de..71776ae 100644
--- a/src/debugger.cc
+++ b/src/debugger.cc
@@ -1355,22 +1355,12 @@
   return gRegistry->Add(parent);
 }
 
-static Object* GetStaticThreadGroup(const char* field_name) {
-  Class* c = Runtime::Current()->GetClassLinker()->FindSystemClass("Ljava/lang/ThreadGroup;");
-  CHECK(c != NULL);
-  Field* f = c->FindStaticField(field_name, "Ljava/lang/ThreadGroup;");
-  CHECK(f != NULL);
-  Object* group = f->GetObject(NULL);
-  CHECK(group != NULL);
-  return group;
-}
-
 JDWP::ObjectId Dbg::GetSystemThreadGroupId() {
-  return gRegistry->Add(GetStaticThreadGroup("mSystem"));
+  return gRegistry->Add(Thread::GetSystemThreadGroup());
 }
 
 JDWP::ObjectId Dbg::GetMainThreadGroupId() {
-  return gRegistry->Add(GetStaticThreadGroup("mMain"));
+  return gRegistry->Add(Thread::GetMainThreadGroup());
 }
 
 bool Dbg::GetThreadStatus(JDWP::ObjectId threadId, JDWP::JdwpThreadStatus* pThreadStatus, JDWP::JdwpSuspendStatus* pSuspendStatus) {
diff --git a/src/jdwp/jdwp_main.cc b/src/jdwp/jdwp_main.cc
index f31eb0b..8fa650a 100644
--- a/src/jdwp/jdwp_main.cc
+++ b/src/jdwp/jdwp_main.cc
@@ -269,7 +269,7 @@
 
 void JdwpState::Run() {
   Runtime* runtime = Runtime::Current();
-  runtime->AttachCurrentThread("JDWP", true);
+  runtime->AttachCurrentThread("JDWP", true, Thread::GetSystemThreadGroup());
 
   VLOG(jdwp) << "JDWP: thread running";
 
diff --git a/src/jni_internal.cc b/src/jni_internal.cc
index dbee18b..f0943a7 100644
--- a/src/jni_internal.cc
+++ b/src/jni_internal.cc
@@ -485,12 +485,12 @@
   return buffer_class;
 }
 
-static jint JII_AttachCurrentThread(JavaVM* vm, JNIEnv** p_env, void* thr_args, bool as_daemon) {
+static jint JII_AttachCurrentThread(JavaVM* vm, JNIEnv** p_env, void* raw_args, bool as_daemon) {
   if (vm == NULL || p_env == NULL) {
     return JNI_ERR;
   }
 
-  // Return immediately if we're already one with the VM.
+  // Return immediately if we're already attached.
   Thread* self = Thread::Current();
   if (self != NULL) {
     *p_env = self->GetJniEnv();
@@ -505,26 +505,16 @@
     return JNI_ERR;
   }
 
-  JavaVMAttachArgs* in_args = static_cast<JavaVMAttachArgs*>(thr_args);
-  JavaVMAttachArgs args;
-  if (thr_args == NULL) {
-    // Allow the v1.1 calling convention.
-    args.version = JNI_VERSION_1_2;
-    args.name = NULL;
-    args.group = NULL; // TODO: get "main" thread group
-  } else {
-    args.version = in_args->version;
-    args.name = in_args->name;
-    if (in_args->group != NULL) {
-      UNIMPLEMENTED(WARNING) << "thr_args->group != NULL";
-      args.group = NULL; // TODO: decode in_args->group
-    } else {
-      args.group = NULL; // TODO: get "main" thread group
-    }
+  JavaVMAttachArgs* args = static_cast<JavaVMAttachArgs*>(raw_args);
+  const char* thread_name = NULL;
+  Object* thread_group = NULL;
+  if (args != NULL) {
+    CHECK_GE(args->version, JNI_VERSION_1_2);
+    thread_name = args->name;
+    thread_group = static_cast<Thread*>(NULL)->DecodeJObject(args->group);
   }
-  CHECK_GE(args.version, JNI_VERSION_1_2);
 
-  runtime->AttachCurrentThread(args.name, as_daemon);
+  runtime->AttachCurrentThread(thread_name, as_daemon, thread_group);
   *p_env = Thread::Current()->GetJniEnv();
   return JNI_OK;
 }
diff --git a/src/runtime.cc b/src/runtime.cc
index d07a6e5..d2e8e07 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -651,8 +651,8 @@
   Thread::Startup();
 
   // ClassLinker needs an attached thread, but we can't fully attach a thread
-  // without creating objects.
-  Thread::Attach(this, "main", false);
+  // without creating objects. We can't supply a thread group yet; it will be fixed later.
+  Thread::Attach("main", false, NULL);
 
   // Set us to runnable so tools using a runtime can allocate and GC by default
   Thread::Current()->SetState(Thread::kRunnable);
@@ -824,8 +824,8 @@
   CHECK_EQ(sigprocmask(SIG_BLOCK, &sigset, NULL), 0);
 }
 
-void Runtime::AttachCurrentThread(const char* name, bool as_daemon) {
-  Thread::Attach(instance_, name, as_daemon);
+void Runtime::AttachCurrentThread(const char* thread_name, bool as_daemon, Object* thread_group) {
+  Thread::Attach(thread_name, as_daemon, thread_group);
 }
 
 void Runtime::DetachCurrentThread() {
diff --git a/src/runtime.h b/src/runtime.h
index 4bbe54e..36dd5c6 100644
--- a/src/runtime.h
+++ b/src/runtime.h
@@ -123,8 +123,8 @@
   // that the native stack trace we get may point at the wrong call site.
   static void Abort(const char* file, int line);
 
-  // Attaches the current native thread to the runtime.
-  void AttachCurrentThread(const char* name, bool as_daemon);
+  // Attaches the calling native thread to the runtime.
+  void AttachCurrentThread(const char* thread_name, bool as_daemon, Object* thread_group);
 
   void CallExitHook(jint status);
 
diff --git a/src/signal_catcher.cc b/src/signal_catcher.cc
index bc358ed..aad6a08 100644
--- a/src/signal_catcher.cc
+++ b/src/signal_catcher.cc
@@ -167,7 +167,7 @@
   CHECK(signal_catcher != NULL);
 
   Runtime* runtime = Runtime::Current();
-  runtime->AttachCurrentThread("Signal Catcher", true);
+  runtime->AttachCurrentThread("Signal Catcher", true, Thread::GetSystemThreadGroup());
   Thread::Current()->SetState(Thread::kRunnable);
 
   {
diff --git a/src/thread.cc b/src/thread.cc
index 7faf8f5..5499091 100644
--- a/src/thread.cc
+++ b/src/thread.cc
@@ -53,6 +53,7 @@
 
 pthread_key_t Thread::pthread_key_self_;
 
+static Class* gThreadGroup = NULL;
 static Class* gThreadLock = NULL;
 static Field* gThread_daemon = NULL;
 static Field* gThread_group = NULL;
@@ -61,6 +62,8 @@
 static Field* gThread_priority = NULL;
 static Field* gThread_uncaughtHandler = NULL;
 static Field* gThread_vmData = NULL;
+static Field* gThreadGroup_mMain = NULL;
+static Field* gThreadGroup_mSystem = NULL;
 static Field* gThreadGroup_name = NULL;
 static Field* gThreadLock_thread = NULL;
 static Method* gThread_run = NULL;
@@ -278,12 +281,11 @@
 
 void* Thread::CreateCallback(void* arg) {
   Thread* self = reinterpret_cast<Thread*>(arg);
-  Runtime* runtime = Runtime::Current();
-
-  self->Attach(runtime);
+  self->Init();
 
   // Wait until it's safe to start running code. (There may have been a suspend-all
   // in progress while we were starting up.)
+  Runtime* runtime = Runtime::Current();
   runtime->GetThreadList()->WaitForGo();
 
   {
@@ -367,14 +369,21 @@
   Runtime::Current()->GetThreadList()->SignalGo(native_thread);
 }
 
-void Thread::Attach(const Runtime* runtime) {
+void Thread::Init() {
+  // This function does all the initialization that must be run by the native thread it applies to.
+  // (When we create a new thread from managed code, we allocate the Thread* in Thread::Create so
+  // we can handshake with the corresponding native thread when it's ready.) Check this native
+  // thread hasn't been through here already...
   CHECK(Thread::Current() == NULL);
 
   InitCpu();
   InitFunctionPointers();
   InitCardTable();
 
-  thin_lock_id_ = Runtime::Current()->GetThreadList()->AllocThreadId();
+  Runtime* runtime = Runtime::Current();
+  CHECK(runtime != NULL);
+
+  thin_lock_id_ = runtime->GetThreadList()->AllocThreadId();
 
   InitTid();
   InitStackHwm();
@@ -386,9 +395,9 @@
   runtime->GetThreadList()->Register();
 }
 
-Thread* Thread::Attach(const Runtime* runtime, const char* name, bool as_daemon) {
+Thread* Thread::Attach(const char* thread_name, bool as_daemon, Object* thread_group) {
   Thread* self = new Thread;
-  self->Attach(runtime);
+  self->Init();
 
   self->SetState(Thread::kNative);
 
@@ -397,29 +406,39 @@
   // In the compiler, all threads need this hack, because no-one's going to be getting
   // a native peer!
   if (self->thin_lock_id_ != ThreadList::kMainId && !Runtime::Current()->IsCompiler()) {
-    self->CreatePeer(name, as_daemon);
+    self->CreatePeer(thread_name, as_daemon, thread_group);
   } else {
     // These aren't necessary, but they improve diagnostics for unit tests & command-line tools.
-    self->name_->assign(name);
-    ::art::SetThreadName(name);
+    self->name_->assign(thread_name);
+    ::art::SetThreadName(thread_name);
   }
 
   self->GetJniEnv()->locals.AssertEmpty();
   return self;
 }
 
-jobject GetWellKnownThreadGroup(JNIEnv* env, const char* field_name) {
-  ScopedLocalRef<jclass> thread_group_class(env, env->FindClass("java/lang/ThreadGroup"));
-  jfieldID fid = env->GetStaticFieldID(thread_group_class.get(), field_name, "Ljava/lang/ThreadGroup;");
-  return env->GetStaticObjectField(thread_group_class.get(), fid);
+Object* Thread::GetMainThreadGroup() {
+  if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(gThreadGroup, true)) {
+    return NULL;
+  }
+  return gThreadGroup_mMain->GetObject(NULL);
 }
 
-void Thread::CreatePeer(const char* name, bool as_daemon) {
+Object* Thread::GetSystemThreadGroup() {
+  if (!Runtime::Current()->GetClassLinker()->EnsureInitialized(gThreadGroup, true)) {
+    return NULL;
+  }
+  return gThreadGroup_mSystem->GetObject(NULL);
+}
+
+void Thread::CreatePeer(const char* name, bool as_daemon, Object* thread_group) {
   CHECK(Runtime::Current()->IsStarted());
   JNIEnv* env = jni_env_;
 
-  const char* field_name = (GetThinLockId() == ThreadList::kMainId) ? "mMain" : "mSystem";
-  ScopedLocalRef<jobject> thread_group(env, GetWellKnownThreadGroup(env, field_name));
+  if (thread_group == NULL) {
+    thread_group = Thread::GetMainThreadGroup();
+  }
+  ScopedLocalRef<jobject> java_thread_group(env, AddLocalReference<jobject>(env, thread_group));
   ScopedLocalRef<jobject> thread_name(env, env->NewStringUTF(name));
   jint thread_priority = GetNativePriority();
   jboolean thread_is_daemon = as_daemon;
@@ -432,7 +451,7 @@
     return;
   }
   jmethodID mid = env->GetMethodID(c.get(), "<init>", "(Ljava/lang/ThreadGroup;Ljava/lang/String;IZ)V");
-  env->CallNonvirtualVoidMethod(peer.get(), c.get(), mid, thread_group.get(), thread_name.get(), thread_priority, thread_is_daemon);
+  env->CallNonvirtualVoidMethod(peer.get(), c.get(), mid, java_thread_group.get(), thread_name.get(), thread_priority, thread_is_daemon);
   CHECK(!IsExceptionPending()) << " " << PrettyTypeOf(GetException());
   SetVmData(peer_, Thread::Current());
 
@@ -443,7 +462,7 @@
     // available (in the compiler, in tests), we manually assign the
     // fields the constructor should have set.
     gThread_daemon->SetBoolean(peer_, thread_is_daemon);
-    gThread_group->SetObject(peer_, Decode<Object*>(env, thread_group.get()));
+    gThread_group->SetObject(peer_, thread_group);
     gThread_name->SetObject(peer_, Decode<Object*>(env, thread_name.get()));
     gThread_priority->SetInt(peer_, thread_priority);
     peer_thread_name.reset(GetThreadName());
@@ -848,6 +867,13 @@
   return m;
 }
 
+// TODO: make more accessible?
+Field* FindStaticFieldOrDie(Class* c, const char* name, const char* descriptor) {
+  Field* f = c->FindDeclaredStaticField(name, descriptor);
+  CHECK(f != NULL) << PrettyClass(c) << " " << name << " " << descriptor;
+  return f;
+}
+
 void Thread::FinishStartup() {
   CHECK(Runtime::Current()->IsStarted());
   Thread* self = Thread::Current();
@@ -859,8 +885,8 @@
   ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
 
   Class* Thread_class = FindClassOrDie(class_linker, "Ljava/lang/Thread;");
-  Class* ThreadGroup_class = FindClassOrDie(class_linker, "Ljava/lang/ThreadGroup;");
   Class* UncaughtExceptionHandler_class = FindClassOrDie(class_linker, "Ljava/lang/Thread$UncaughtExceptionHandler;");
+  gThreadGroup = FindClassOrDie(class_linker, "Ljava/lang/ThreadGroup;");
   gThreadLock = FindClassOrDie(class_linker, "Ljava/lang/ThreadLock;");
 
   gThread_daemon = FindFieldOrDie(Thread_class, "daemon", "Z");
@@ -870,16 +896,18 @@
   gThread_priority = FindFieldOrDie(Thread_class, "priority", "I");
   gThread_uncaughtHandler = FindFieldOrDie(Thread_class, "uncaughtHandler", "Ljava/lang/Thread$UncaughtExceptionHandler;");
   gThread_vmData = FindFieldOrDie(Thread_class, "vmData", "I");
-  gThreadGroup_name = FindFieldOrDie(ThreadGroup_class, "name", "Ljava/lang/String;");
+  gThreadGroup_name = FindFieldOrDie(gThreadGroup, "name", "Ljava/lang/String;");
+  gThreadGroup_mMain = FindStaticFieldOrDie(gThreadGroup, "mMain", "Ljava/lang/ThreadGroup;");
+  gThreadGroup_mSystem = FindStaticFieldOrDie(gThreadGroup, "mSystem", "Ljava/lang/ThreadGroup;");
   gThreadLock_thread = FindFieldOrDie(gThreadLock, "thread", "Ljava/lang/Thread;");
 
   gThread_run = FindMethodOrDie(Thread_class, "run", "()V");
-  gThreadGroup_removeThread = FindMethodOrDie(ThreadGroup_class, "removeThread", "(Ljava/lang/Thread;)V");
+  gThreadGroup_removeThread = FindMethodOrDie(gThreadGroup, "removeThread", "(Ljava/lang/Thread;)V");
   gUncaughtExceptionHandler_uncaughtException = FindMethodOrDie(UncaughtExceptionHandler_class,
       "uncaughtException", "(Ljava/lang/Thread;Ljava/lang/Throwable;)V");
 
   // Finish attaching the main thread.
-  Thread::Current()->CreatePeer("main", false);
+  Thread::Current()->CreatePeer("main", false, Thread::GetMainThreadGroup());
 
   InitBoxingMethods();
   class_linker->RunRootClinits();
diff --git a/src/thread.h b/src/thread.h
index 7bf9c41..ece54a2 100644
--- a/src/thread.h
+++ b/src/thread.h
@@ -96,11 +96,13 @@
     virtual bool VisitFrame(const Frame& frame, uintptr_t pc) = 0;
   };
 
-  // Creates a new thread.
+  // Creates a new native thread corresponding to the given managed peer.
+  // Used to implement Thread.start.
   static void Create(Object* peer, size_t stack_size);
 
-  // Creates a new thread from the calling thread.
-  static Thread* Attach(const Runtime* runtime, const char* name, bool as_daemon);
+  // Attaches the calling native thread to the runtime, returning the new native peer.
+  // Used to implement JNI AttachCurrentThread and AttachCurrentThreadAsDaemon calls.
+  static Thread* Attach(const char* thread_name, bool as_daemon, Object* thread_group);
 
   // Reset internal state of child thread after fork.
   void InitAfterFork();
@@ -148,6 +150,11 @@
    */
   static int GetNativePriority();
 
+  // Returns the "main" ThreadGroup, used when attaching user threads.
+  static Object* GetMainThreadGroup();
+  // Returns the "system" ThreadGroup, used when attaching our internal threads.
+  static Object* GetSystemThreadGroup();
+
   bool CanAccessDirectReferences() const {
 #ifdef MOVING_GARBAGE_COLLECTOR
     // TODO: when we have a moving collector, we'll need: return state_ == kRunnable;
@@ -438,7 +445,7 @@
   ~Thread();
   friend class ThreadList;  // For ~Thread.
 
-  void CreatePeer(const char* name, bool as_daemon);
+  void CreatePeer(const char* name, bool as_daemon, Object* thread_group);
   friend class Runtime; // For CreatePeer.
 
   void DumpState(std::ostream& os) const;
@@ -449,12 +456,12 @@
   static Thread* CurrentFromGdb(); // Like Thread::Current.
   void DumpFromGdb() const; // Like Thread::Dump(std::cerr).
 
-  void Attach(const Runtime* runtime);
   static void* CreateCallback(void* arg);
 
   void HandleUncaughtExceptions();
   void RemoveFromThreadGroup();
 
+  void Init();
   void InitCardTable();
   void InitCpu();
   void InitFunctionPointers();