ART: Call ThreadGroup.add for attached threads
When attaching threads to the runtime, call ThreadGroup.add to
let the thread's group know that the thread is now started. This
fixes incorrect internal accounting.
Test: art/test/testrunner/testrunner.py -b --host -t 169
Test: m test-art-host
Change-Id: I60362b6b53acf06b97779ea9b3b0bc6264f1dc2d
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 6d065d6..77f1fb9 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1855,7 +1855,13 @@
bool Runtime::AttachCurrentThread(const char* thread_name, bool as_daemon, jobject thread_group,
bool create_peer) {
ScopedTrace trace(__FUNCTION__);
- return Thread::Attach(thread_name, as_daemon, thread_group, create_peer) != nullptr;
+ Thread* self = Thread::Attach(thread_name, as_daemon, thread_group, create_peer);
+ // Run ThreadGroup.add to notify the group that this thread is now started.
+ if (self != nullptr && create_peer && !IsAotCompiler()) {
+ ScopedObjectAccess soa(self);
+ self->NotifyThreadGroup(soa, thread_group);
+ }
+ return self != nullptr;
}
void Runtime::DetachCurrentThread() {
diff --git a/runtime/thread.cc b/runtime/thread.cc
index 3fe954c..9dc92f3 100644
--- a/runtime/thread.cc
+++ b/runtime/thread.cc
@@ -2049,20 +2049,8 @@
// The thread counts as started from now on. We need to add it to the ThreadGroup. For regular
// threads, this is done in Thread.start() on the Java side.
- {
- // This is only ever done once. There's no benefit in caching the method.
- jmethodID thread_group_add = soa.Env()->GetMethodID(WellKnownClasses::java_lang_ThreadGroup,
- "add",
- "(Ljava/lang/Thread;)V");
- CHECK(thread_group_add != nullptr);
- ScopedLocalRef<jobject> thread_jobject(
- soa.Env(), soa.Env()->AddLocalReference<jobject>(Thread::Current()->GetPeer()));
- soa.Env()->CallNonvirtualVoidMethod(runtime->GetMainThreadGroup(),
- WellKnownClasses::java_lang_ThreadGroup,
- thread_group_add,
- thread_jobject.get());
- Thread::Current()->AssertNoPendingException();
- }
+ Thread::Current()->NotifyThreadGroup(soa, runtime->GetMainThreadGroup());
+ Thread::Current()->AssertNoPendingException();
}
void Thread::Shutdown() {
@@ -2076,6 +2064,28 @@
}
}
+void Thread::NotifyThreadGroup(ScopedObjectAccessAlreadyRunnable& soa, jobject thread_group) {
+ ScopedLocalRef<jobject> thread_jobject(
+ soa.Env(), soa.Env()->AddLocalReference<jobject>(Thread::Current()->GetPeer()));
+ ScopedLocalRef<jobject> thread_group_jobject_scoped(
+ soa.Env(), nullptr);
+ jobject thread_group_jobject = thread_group;
+ if (thread_group == nullptr || kIsDebugBuild) {
+ // There is always a group set. Retrieve it.
+ thread_group_jobject_scoped.reset(
+ soa.Env()->GetObjectField(thread_jobject.get(),
+ WellKnownClasses::java_lang_Thread_group));
+ thread_group_jobject = thread_group_jobject_scoped.get();
+ if (kIsDebugBuild && thread_group != nullptr) {
+ CHECK(soa.Env()->IsSameObject(thread_group, thread_group_jobject));
+ }
+ }
+ soa.Env()->CallNonvirtualVoidMethod(thread_group_jobject,
+ WellKnownClasses::java_lang_ThreadGroup,
+ WellKnownClasses::java_lang_ThreadGroup_add,
+ thread_jobject.get());
+}
+
Thread::Thread(bool daemon)
: tls32_(daemon),
wait_monitor_(nullptr),
diff --git a/runtime/thread.h b/runtime/thread.h
index 426d27d..295685e 100644
--- a/runtime/thread.h
+++ b/runtime/thread.h
@@ -511,6 +511,12 @@
static void FinishStartup();
static void Shutdown();
+ // Notify this thread's thread-group that this thread has started.
+ // Note: the given thread-group is used as a fast path and verified in debug build. If the value
+ // is null, the thread's thread-group is loaded from the peer.
+ void NotifyThreadGroup(ScopedObjectAccessAlreadyRunnable& soa, jobject thread_group = nullptr)
+ REQUIRES_SHARED(Locks::mutator_lock_);
+
// JNI methods
JNIEnvExt* GetJniEnv() const {
return tlsPtr_.jni_env;
diff --git a/runtime/well_known_classes.cc b/runtime/well_known_classes.cc
index dc57f81..5fe10f5 100644
--- a/runtime/well_known_classes.cc
+++ b/runtime/well_known_classes.cc
@@ -110,6 +110,7 @@
jmethodID WellKnownClasses::java_lang_Thread_dispatchUncaughtException;
jmethodID WellKnownClasses::java_lang_Thread_init;
jmethodID WellKnownClasses::java_lang_Thread_run;
+jmethodID WellKnownClasses::java_lang_ThreadGroup_add;
jmethodID WellKnownClasses::java_lang_ThreadGroup_removeThread;
jmethodID WellKnownClasses::java_nio_DirectByteBuffer_init;
jmethodID WellKnownClasses::libcore_reflect_AnnotationFactory_createAnnotation;
@@ -347,6 +348,7 @@
java_lang_Thread_dispatchUncaughtException = CacheMethod(env, java_lang_Thread, false, "dispatchUncaughtException", "(Ljava/lang/Throwable;)V");
java_lang_Thread_init = CacheMethod(env, java_lang_Thread, false, "<init>", "(Ljava/lang/ThreadGroup;Ljava/lang/String;IZ)V");
java_lang_Thread_run = CacheMethod(env, java_lang_Thread, false, "run", "()V");
+ java_lang_ThreadGroup_add = CacheMethod(env, java_lang_ThreadGroup, false, "add", "(Ljava/lang/Thread;)V");
java_lang_ThreadGroup_removeThread = CacheMethod(env, java_lang_ThreadGroup, false, "threadTerminated", "(Ljava/lang/Thread;)V");
java_nio_DirectByteBuffer_init = CacheMethod(env, java_nio_DirectByteBuffer, false, "<init>", "(JI)V");
libcore_reflect_AnnotationFactory_createAnnotation = CacheMethod(env, libcore_reflect_AnnotationFactory, true, "createAnnotation", "(Ljava/lang/Class;[Llibcore/reflect/AnnotationMember;)Ljava/lang/annotation/Annotation;");
@@ -496,6 +498,7 @@
java_lang_Thread_dispatchUncaughtException = nullptr;
java_lang_Thread_init = nullptr;
java_lang_Thread_run = nullptr;
+ java_lang_ThreadGroup_add = nullptr;
java_lang_ThreadGroup_removeThread = nullptr;
java_nio_DirectByteBuffer_init = nullptr;
libcore_reflect_AnnotationFactory_createAnnotation = nullptr;
diff --git a/runtime/well_known_classes.h b/runtime/well_known_classes.h
index 024971a..9e0b079 100644
--- a/runtime/well_known_classes.h
+++ b/runtime/well_known_classes.h
@@ -121,6 +121,7 @@
static jmethodID java_lang_Thread_dispatchUncaughtException;
static jmethodID java_lang_Thread_init;
static jmethodID java_lang_Thread_run;
+ static jmethodID java_lang_ThreadGroup_add;
static jmethodID java_lang_ThreadGroup_removeThread;
static jmethodID java_nio_DirectByteBuffer_init;
static jmethodID libcore_reflect_AnnotationFactory_createAnnotation;
diff --git a/test/169-threadgroup-jni/expected.txt b/test/169-threadgroup-jni/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/169-threadgroup-jni/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/169-threadgroup-jni/info.txt b/test/169-threadgroup-jni/info.txt
new file mode 100644
index 0000000..b4c77e2
--- /dev/null
+++ b/test/169-threadgroup-jni/info.txt
@@ -0,0 +1 @@
+Ensure that attached threads are correctly handled in ThreadGroups.
diff --git a/test/169-threadgroup-jni/jni_daemon_thread.cc b/test/169-threadgroup-jni/jni_daemon_thread.cc
new file mode 100644
index 0000000..94902dc
--- /dev/null
+++ b/test/169-threadgroup-jni/jni_daemon_thread.cc
@@ -0,0 +1,65 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <jni.h>
+#include <nativehelper/scoped_local_ref.h>
+#include <pthread.h>
+
+#include <android-base/logging.h>
+
+namespace art {
+
+static JavaVM* vm = nullptr;
+
+static void* Runner(void* arg) {
+ CHECK(vm != nullptr);
+
+ jobject thread_group = reinterpret_cast<jobject>(arg);
+ JNIEnv* env = nullptr;
+ JavaVMAttachArgs args = { JNI_VERSION_1_6, __FUNCTION__, thread_group };
+ int attach_result = vm->AttachCurrentThread(&env, &args);
+ CHECK_EQ(attach_result, 0);
+
+ {
+ ScopedLocalRef<jclass> klass(env, env->FindClass("Main"));
+ CHECK(klass != nullptr);
+
+ jmethodID id = env->GetStaticMethodID(klass.get(), "runFromNative", "()V");
+ CHECK(id != nullptr);
+
+ env->CallStaticVoidMethod(klass.get(), id);
+ }
+
+ int detach_result = vm->DetachCurrentThread();
+ CHECK_EQ(detach_result, 0);
+ return nullptr;
+}
+
+extern "C" JNIEXPORT void JNICALL Java_Main_testNativeThread(
+ JNIEnv* env, jclass, jobject thread_group) {
+ CHECK_EQ(env->GetJavaVM(&vm), 0);
+ jobject global_thread_group = env->NewGlobalRef(thread_group);
+
+ pthread_t pthread;
+ int pthread_create_result = pthread_create(&pthread, nullptr, Runner, global_thread_group);
+ CHECK_EQ(pthread_create_result, 0);
+ int pthread_join_result = pthread_join(pthread, nullptr);
+ CHECK_EQ(pthread_join_result, 0);
+
+ env->DeleteGlobalRef(global_thread_group);
+}
+
+} // namespace art
diff --git a/test/169-threadgroup-jni/src/Main.java b/test/169-threadgroup-jni/src/Main.java
new file mode 100644
index 0000000..2cd1fcf
--- /dev/null
+++ b/test/169-threadgroup-jni/src/Main.java
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+
+ ThreadGroup group = new ThreadGroup("Test group");
+ group.setDaemon(true);
+
+ testNativeThread(group);
+
+ if (!executed) {
+ throw new IllegalStateException("Expected runFromNative to be done.");
+ }
+ if (!group.isDestroyed()) {
+ throw new IllegalStateException("Threadgroup should be destroyed.");
+ }
+ }
+
+ private static boolean executed = false;
+ private static void runFromNative() {
+ executed = true;
+ }
+ private static native void testNativeThread(ThreadGroup group);
+}
diff --git a/test/Android.bp b/test/Android.bp
index 2985077..72e8eee 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -373,6 +373,7 @@
"149-suspend-all-stress/suspend_all.cc",
"154-gc-loop/heap_interface.cc",
"167-visit-locks/visit_locks.cc",
+ "169-threadgroup-jni/jni_daemon_thread.cc",
"203-multi-checkpoint/multi_checkpoint.cc",
"305-other-fault-handler/fault_handler.cc",
"454-get-vreg/get_vreg_jni.cc",