Thread: Don't set native names twice for attached threads.
ART already does this when a threads are attached. There's no need
to do it another time.
Also add a test that documents the behaviour wrt. to thread naming
for attached threads.
bug: 27748318
Change-Id: I9075f0434b0168280662a1f8a4176f23724748a7
diff --git a/NativeCode.mk b/NativeCode.mk
index 7ab8e4f..be02ff1 100644
--- a/NativeCode.mk
+++ b/NativeCode.mk
@@ -94,6 +94,7 @@
core_test_files := \
luni/src/test/native/dalvik_system_JniTest.cpp \
luni/src/test/native/libcore_java_io_FileTest.cpp \
+ luni/src/test/native/libcore_java_lang_ThreadTest.cpp \
luni/src/test/native/libcore_java_nio_BufferTest.cpp \
luni/src/test/native/libcore_util_NativeAllocationRegistryTest.cpp \
diff --git a/luni/src/test/java/libcore/java/lang/ThreadTest.java b/luni/src/test/java/libcore/java/lang/ThreadTest.java
index 8545a20..10ca9a7 100644
--- a/luni/src/test/java/libcore/java/lang/ThreadTest.java
+++ b/luni/src/test/java/libcore/java/lang/ThreadTest.java
@@ -22,6 +22,9 @@
import libcore.java.lang.ref.FinalizationTester;
public final class ThreadTest extends TestCase {
+ static {
+ System.loadLibrary("javacoretests");
+ }
/**
* getContextClassLoader returned a non-application class loader.
@@ -167,6 +170,19 @@
assertTrue("Must have traces for all threads", visibleTraces.get() > 1);
}
+ // http://b/27748318
+ public void testNativeThreadNames() throws Exception {
+ String testResult = nativeTestNativeThreadNames();
+ // Not using assertNull here because this results in a better error message.
+ if (testResult != null) {
+ fail(testResult);
+ }
+ }
+
+ // This method returns {@code null} if all tests pass, or a non-null String containing
+ // failure details if an error occured.
+ private static native String nativeTestNativeThreadNames();
+
private Thread newThread(final AtomicInteger finalizedThreadsCount, final int size) {
return new Thread() {
long[] memoryPressure = new long[size];
diff --git a/luni/src/test/native/libcore_java_lang_ThreadTest.cpp b/luni/src/test/native/libcore_java_lang_ThreadTest.cpp
new file mode 100644
index 0000000..3e52d997
--- /dev/null
+++ b/luni/src/test/native/libcore_java_lang_ThreadTest.cpp
@@ -0,0 +1,134 @@
+/*
+ * Copyright (C) 2015 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 <memory>
+#include <string>
+
+#include <pthread.h>
+#include <sys/prctl.h>
+
+#include <jni.h>
+#include "JNIHelp.h"
+
+static JavaVM* javaVm = nullptr;
+
+static void* TestThreadNaming(void* arg) {
+ const bool attach_with_name = (reinterpret_cast<uint64_t>(arg) == 1);
+ const std::string native_thread_name = "foozball";
+ pthread_setname_np(pthread_self(), native_thread_name.c_str());
+
+ JNIEnv* env;
+ JavaVMAttachArgs args;
+ args.version = JNI_VERSION_1_6;
+ args.group = nullptr;
+ if (attach_with_name) {
+ args.name = native_thread_name.c_str();
+ }
+
+ if (javaVm->AttachCurrentThread(&env, &args) != JNI_OK) {
+ return new std::string("Attach failed");
+ }
+
+ std::string* exception_message = nullptr;
+ std::unique_ptr<char[]> thread_name(new char[32]);
+ if (prctl(PR_GET_NAME, reinterpret_cast<unsigned long>(thread_name.get()), 0L, 0L, 0L) == 0) {
+ // If a thread is attached with a name, the native thread name must be set to
+ // the supplied name. In this test, the name we attach with == the
+ // native_thread_name.
+ if (attach_with_name && (thread_name.get() != native_thread_name)) {
+ exception_message = new std::string("expected_thread_name != thread_name: ");
+ exception_message->append("expected :");
+ exception_message->append(native_thread_name);
+ exception_message->append(" was :");
+ exception_message->append(thread_name.get());
+ }
+
+ // On the other hand, if the thread isn't attached with a name - the
+ // runtime assigns a name according to the usual thread naming scheme.
+ if (!attach_with_name && strncmp(thread_name.get(), "Thread", 6)) {
+ exception_message = new std::string("unexpected thread name : ");
+ exception_message->append(thread_name.get());
+ }
+ } else {
+ exception_message = new std::string("prctl(PR_GET_NAME) failed :");
+ exception_message->append(strerror(errno));
+ }
+
+
+ if (javaVm->DetachCurrentThread() != JNI_OK) {
+ exception_message = new std::string("Detach failed");
+ }
+
+ return exception_message;
+}
+
+extern "C" jstring Java_libcore_java_lang_ThreadTest_nativeTestNativeThreadNames(
+ JNIEnv* env, jobject /* object */) {
+ std::string result;
+
+ // TEST 1: Test that a thread attaching with a specified name (in the
+ // JavaVMAttachArgs) does not have its name changed.
+ pthread_t attacher;
+ if (pthread_create(&attacher, nullptr, TestThreadNaming,
+ reinterpret_cast<void*>(static_cast<uint64_t>(0))) != 0) {
+ jniThrowException(env, "java/lang/IllegalStateException", "Attach failed");
+ }
+
+ std::string* result_test1;
+ if (pthread_join(attacher, reinterpret_cast<void**>(&result_test1)) != 0) {
+ jniThrowException(env, "java/lang/IllegalStateException", "Join failed");
+ }
+
+ if (result_test1 != nullptr) {
+ result.append("test 1: ");
+ result.append(*result_test1);
+ }
+
+ // TEST 2: Test that a thread attaching without a specified name (in the
+ // JavaVMAttachArgs) has its native name changed as per the standard naming
+ // convention.
+ pthread_t attacher2;
+ if (pthread_create(&attacher2, nullptr, TestThreadNaming,
+ reinterpret_cast<void*>(static_cast<uint64_t>(1))) != 0) {
+ jniThrowException(env, "java/lang/IllegalStateException", "Attach failed");
+ }
+
+ std::string* result_test2;
+ if (pthread_join(attacher2, reinterpret_cast<void**>(&result_test2)) != 0) {
+ jniThrowException(env, "java/lang/IllegalStateException", "Join failed");
+ }
+
+ if (result_test2 != nullptr) {
+ result.append("test 2: ");
+ result.append(*result_test2);
+ }
+
+ // Return test results.
+ jstring resultJString = nullptr;
+ if (result.size() > 0) {
+ resultJString = env->NewStringUTF(result.c_str());
+ }
+
+ delete result_test1;
+ delete result_test2;
+
+ return resultJString;
+}
+
+extern "C" int JNI_OnLoad(JavaVM* vm, void*) {
+ javaVm = vm;
+ return JNI_VERSION_1_6;
+}
diff --git a/ojluni/src/main/java/java/lang/Thread.java b/ojluni/src/main/java/java/lang/Thread.java
index c14022b..1436dd4 100755
--- a/ojluni/src/main/java/java/lang/Thread.java
+++ b/ojluni/src/main/java/java/lang/Thread.java
@@ -395,53 +395,6 @@
* zero to indicate that this parameter is to be ignored.
*/
private void init(ThreadGroup g, Runnable target, String name, long stackSize) {
- // Android changed : Reimplemented.
- /*
- if (name == null) {
- throw new NullPointerException("name cannot be null");
- }
-
- Thread parent = currentThread();
- SecurityManager security = System.getSecurityManager();
- if (g == null) {
- // Determine if it's an applet or not
-
- // If there is a security manager, ask the security manager what to do.
- if (security != null) {
- g = security.getThreadGroup();
- }
-
- // If the security doesn't have a strong opinion of the matter
- // use the parent thread group.
- if (g == null) {
- g = parent.getThreadGroup();
- }
- }
-
- // checkAccess regardless of whether or not threadgroup is explicitly passed in.
- g.checkAccess();
-
- // Do we have the required permissions?
- if (security != null) {
- if (isCCLOverridden(getClass())) {
- security.checkPermission(SUBCLASS_IMPLEMENTATION_PERMISSION);
- }
- }
- if (g == null) {
- g = parent.getThreadGroup();
- }
- g.checkAccess();
- this.daemon = parent.isDaemon();
- this.priority = parent.getPriority();
- this.name = name.toCharArray();
- if (security == null || isCCLOverridden(parent.getClass()))
- this.contextClassLoader = parent.getContextClassLoader();
- else
- this.contextClassLoader = parent.contextClassLoader;
- tid = nextThreadID();
- */
- // ----- END android -----
-
Thread parent = currentThread();
if (g == null) {
g = parent.getThreadGroup();
@@ -574,7 +527,12 @@
if (name == null) {
name = "Thread-" + nextThreadNum();
}
- setName(name);
+
+ // NOTE: Resist the temptation to call setName() here. This constructor is only called
+ // by the runtime to construct peers for threads that have attached via JNI and it's
+ // undesirable to clobber their natively set name.
+ this.name = name;
+
this.priority = priority;
this.daemon = daemon;
init2(currentThread());
