Make sure all runtime threads are in the System thread group.
We want to be sure that one can suspend every thread in the 'main'
ThreadGroup without affecting the ability of the runtime to perform
normal actions. Previously the jit thread-pool created threads in the
'main' thread group. Since some debugger actions can wait until all
jit threads are in a known good state, pausing all threads in the main
thread-group can cause deadlocks. To fix this we make the ThreadPool
create all threads in the system thread-group.
To test this we perform structural redefinition with all threads
except the main one suspended. Previously this would deadlock if run
with --jit, as we will pause the jit before performing redefinition.
Test: ./test.py --host --jit
Test: ./test.py --host
Bug: 146178357
Change-Id: Ifcb0f29613d2fc22ca7913d4868a1e425b0bee5b
diff --git a/runtime/thread_pool.cc b/runtime/thread_pool.cc
index 365c097..2bca5a9 100644
--- a/runtime/thread_pool.cc
+++ b/runtime/thread_pool.cc
@@ -97,10 +97,17 @@
void* ThreadPoolWorker::Callback(void* arg) {
ThreadPoolWorker* worker = reinterpret_cast<ThreadPoolWorker*>(arg);
Runtime* runtime = Runtime::Current();
- CHECK(runtime->AttachCurrentThread(worker->name_.c_str(),
- true,
- nullptr,
- worker->thread_pool_->create_peers_));
+ CHECK(runtime->AttachCurrentThread(
+ worker->name_.c_str(),
+ true,
+ // Thread-groups are only tracked by the peer j.l.Thread objects. If we aren't creating peers
+ // we don't need to specify the thread group. We want to place these threads in the System
+ // thread group because that thread group is where important threads that debuggers and
+ // similar tools should not mess with are placed. As this is an internal-thread-pool we might
+ // rely on being able to (for example) wait for all threads to finish some task. If debuggers
+ // are suspending these threads that might not be possible.
+ worker->thread_pool_->create_peers_ ? runtime->GetSystemThreadGroup() : nullptr,
+ worker->thread_pool_->create_peers_));
worker->thread_ = Thread::Current();
// Mark thread pool workers as runtime-threads.
worker->thread_->SetIsRuntimeThread(true);
diff --git a/test/2005-pause-all-redefine-multithreaded/expected.txt b/test/2005-pause-all-redefine-multithreaded/expected.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/expected.txt
diff --git a/test/2005-pause-all-redefine-multithreaded/info.txt b/test/2005-pause-all-redefine-multithreaded/info.txt
new file mode 100644
index 0000000..c300f0c
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/info.txt
@@ -0,0 +1,5 @@
+Tests structural redefinition with multiple threads.
+
+Tests that using the structural redefinition while pausing all other (main thread-group) threads
+doesn't cause problems. This also tests that we can update the newly created fields while the
+other threads are suspended, thus making them look initialized.
diff --git a/test/2005-pause-all-redefine-multithreaded/pause-all.cc b/test/2005-pause-all-redefine-multithreaded/pause-all.cc
new file mode 100644
index 0000000..ca13d3a
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/pause-all.cc
@@ -0,0 +1,88 @@
+/*
+ * Copyright (C) 2013 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 <stdio.h>
+
+#include <vector>
+
+#include "android-base/logging.h"
+#include "android-base/macros.h"
+#include "jni.h"
+#include "jvmti.h"
+
+// Test infrastructure
+#include "jvmti_helper.h"
+#include "scoped_local_ref.h"
+#include "test_env.h"
+
+namespace art {
+namespace Test2005PauseAllRedefineMultithreaded {
+
+static constexpr jlong kRedefinedObjectTag = 0xDEADBEEF;
+
+extern "C" JNIEXPORT void JNICALL
+Java_art_Test2005_UpdateFieldValuesAndResumeThreads(JNIEnv* env,
+ jclass klass ATTRIBUTE_UNUSED,
+ jobjectArray threads_arr,
+ jclass redefined_class,
+ jobjectArray new_fields,
+ jstring default_val) {
+ std::vector<jthread> threads;
+ for (jint i = 0; i < env->GetArrayLength(threads_arr); i++) {
+ threads.push_back(env->GetObjectArrayElement(threads_arr, i));
+ }
+ std::vector<jfieldID> fields;
+ for (jint i = 0; i < env->GetArrayLength(new_fields); i++) {
+ fields.push_back(env->FromReflectedField(env->GetObjectArrayElement(new_fields, i)));
+ }
+ // Tag every instance of the redefined class with kRedefinedObjectTag
+ CHECK_EQ(jvmti_env->IterateOverInstancesOfClass(
+ redefined_class,
+ JVMTI_HEAP_OBJECT_EITHER,
+ [](jlong class_tag ATTRIBUTE_UNUSED,
+ jlong size ATTRIBUTE_UNUSED,
+ jlong* tag_ptr,
+ void* user_data ATTRIBUTE_UNUSED) -> jvmtiIterationControl {
+ *tag_ptr = kRedefinedObjectTag;
+ return JVMTI_ITERATION_CONTINUE;
+ },
+ nullptr),
+ JVMTI_ERROR_NONE);
+ jobject* objs;
+ jint cnt;
+ // Get the objects.
+ CHECK_EQ(jvmti_env->GetObjectsWithTags(1, &kRedefinedObjectTag, &cnt, &objs, nullptr),
+ JVMTI_ERROR_NONE);
+ // Set every field that's null
+ for (jint i = 0; i < cnt; i++) {
+ jobject obj = objs[i];
+ for (jfieldID field : fields) {
+ if (ScopedLocalRef<jobject>(env, env->GetObjectField(obj, field)).get() == nullptr) {
+ env->SetObjectField(obj, field, default_val);
+ }
+ }
+ }
+ LOG(INFO) << "Setting " << cnt << " objects with default values";
+ if (!threads.empty()) {
+ std::vector<jvmtiError> errs(threads.size(), JVMTI_ERROR_NONE);
+ CHECK_EQ(jvmti_env->ResumeThreadList(threads.size(), threads.data(), errs.data()),
+ JVMTI_ERROR_NONE);
+ }
+ jvmti_env->Deallocate(reinterpret_cast<unsigned char*>(objs));
+}
+
+} // namespace Test2005PauseAllRedefineMultithreaded
+} // namespace art
diff --git a/test/2005-pause-all-redefine-multithreaded/run b/test/2005-pause-all-redefine-multithreaded/run
new file mode 100755
index 0000000..03e41a5
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/run
@@ -0,0 +1,17 @@
+#!/bin/bash
+#
+# Copyright 2016 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.
+
+./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true
diff --git a/test/2005-pause-all-redefine-multithreaded/src/Main.java b/test/2005-pause-all-redefine-multithreaded/src/Main.java
new file mode 100644
index 0000000..72cf037
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/src/Main.java
@@ -0,0 +1,21 @@
+/*
+ * 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 {
+ art.Test2005.run();
+ }
+}
diff --git a/test/2005-pause-all-redefine-multithreaded/src/art/Redefinition.java b/test/2005-pause-all-redefine-multithreaded/src/art/Redefinition.java
new file mode 120000
index 0000000..81eaf31
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/src/art/Redefinition.java
@@ -0,0 +1 @@
+../../../jvmti-common/Redefinition.java
\ No newline at end of file
diff --git a/test/2005-pause-all-redefine-multithreaded/src/art/Suspension.java b/test/2005-pause-all-redefine-multithreaded/src/art/Suspension.java
new file mode 120000
index 0000000..bcef96f
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/src/art/Suspension.java
@@ -0,0 +1 @@
+../../../jvmti-common/Suspension.java
\ No newline at end of file
diff --git a/test/2005-pause-all-redefine-multithreaded/src/art/Test2005.java b/test/2005-pause-all-redefine-multithreaded/src/art/Test2005.java
new file mode 100644
index 0000000..9de7643
--- /dev/null
+++ b/test/2005-pause-all-redefine-multithreaded/src/art/Test2005.java
@@ -0,0 +1,198 @@
+/*
+ * Copyright (C) 2016 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.
+ */
+
+package art;
+
+import java.lang.reflect.Field;
+import java.util.*;
+import java.util.concurrent.CountDownLatch;
+public class Test2005 {
+ private static final int NUM_THREADS = 20;
+ private static final String DEFAULT_VAL = "DEFAULT_VALUE";
+
+ public static final class Transform {
+ public String greetingEnglish;
+ public Transform() {
+ this.greetingEnglish = "Hello";
+ }
+ public String sayHi() {
+ return greetingEnglish + " from " + Thread.currentThread().getName();
+ }
+ }
+
+ /**
+ * base64 encoded class/dex file for
+ * public static final class Transform {
+ * public String greetingEnglish;
+ * public String greetingFrench;
+ * public String greetingDanish;
+ * public String greetingJapanese;
+ *
+ * public Transform() {
+ * this.greetingEnglish = "Hello World";
+ * this.greetingFrench = "Bonjour le Monde";
+ * this.greetingDanish = "Hej Verden";
+ * this.greetingJapanese = "こんにちは世界";
+ * }
+ * public String sayHi() {
+ * return sayHiEnglish() + ", " + sayHiFrench() + ", " + sayHiDanish() + ", " +
+ * sayHiJapanese() + " from " + Thread.currentThread().getName();
+ * }
+ * public String sayHiEnglish() {
+ * return greetingEnglish;
+ * }
+ * public String sayHiDanish() {
+ * return greetingDanish;
+ * }
+ * public String sayHiJapanese() {
+ * return greetingJapanese;
+ * }
+ * public String sayHiFrench() {
+ * return greetingFrench;
+ * }
+ * }
+ */
+ private static final byte[] DEX_BYTES = Base64.getDecoder().decode(
+ "ZGV4CjAzNQAgJ1QXHJ8PAODMKTV14wyH4oKGOMK1yyL4BgAAcAAAAHhWNBIAAAAAAAAAADQGAAAl"
+ + "AAAAcAAAAAkAAAAEAQAABAAAACgBAAAEAAAAWAEAAAwAAAB4AQAAAQAAANgBAAAABQAA+AEAAEoD"
+ + "AABSAwAAVgMAAF4DAABwAwAAfAMAAIkDAACMAwAAkAMAAKoDAAC6AwAA3gMAAP4DAAASBAAAJgQA"
+ + "AEEEAABVBAAAZAQAAG8EAAByBAAAfwQAAIcEAACWBAAAnwQAAK8EAADABAAA0AQAAOIEAADoBAAA"
+ + "7wQAAPwEAAAKBQAAFwUAACYFAAAwBQAANwUAAMUFAAAIAAAACQAAAAoAAAALAAAADAAAAA0AAAAO"
+ + "AAAADwAAABIAAAAGAAAABQAAAAAAAAAHAAAABgAAAEQDAAAGAAAABwAAAAAAAAASAAAACAAAAAAA"
+ + "AAAAAAUAFwAAAAAABQAYAAAAAAAFABkAAAAAAAUAGgAAAAAAAwACAAAAAAAAABwAAAAAAAAAHQAA"
+ + "AAAAAAAeAAAAAAAAAB8AAAAAAAAAIAAAAAQAAwACAAAABgADAAIAAAAGAAEAFAAAAAYAAAAhAAAA"
+ + "BwACABUAAAAHAAAAFgAAAAAAAAARAAAABAAAAAAAAAAQAAAAJAYAAOsFAAAAAAAABwABAAIAAAAt"
+ + "AwAAQQAAAG4QAwAGAAwAbhAEAAYADAFuEAIABgAMAm4QBQAGAAwDcQAKAAAADARuEAsABAAMBCIF"
+ + "BgBwEAcABQBuIAgABQAaAAEAbiAIAAUAbiAIABUAbiAIAAUAbiAIACUAbiAIAAUAbiAIADUAGgAA"
+ + "AG4gCAAFAG4gCABFAG4QCQAFAAwAEQAAAAIAAQAAAAAAMQMAAAMAAABUEAAAEQAAAAIAAQAAAAAA"
+ + "NQMAAAMAAABUEAEAEQAAAAIAAQAAAAAAOQMAAAMAAABUEAIAEQAAAAIAAQAAAAAAPQMAAAMAAABU"
+ + "EAMAEQAAAAIAAQABAAAAJAMAABQAAABwEAYAAQAaAAUAWxABABoAAwBbEAIAGgAEAFsQAAAaACQA"
+ + "WxADAA4ACQAOPEtLS0sAEAAOABYADgATAA4AHAAOABkADgAAAAABAAAABQAGIGZyb20gAAIsIAAG"
+ + "PGluaXQ+ABBCb25qb3VyIGxlIE1vbmRlAApIZWogVmVyZGVuAAtIZWxsbyBXb3JsZAABTAACTEwA"
+ + "GExhcnQvVGVzdDIwMDUkVHJhbnNmb3JtOwAOTGFydC9UZXN0MjAwNTsAIkxkYWx2aWsvYW5ub3Rh"
+ + "dGlvbi9FbmNsb3NpbmdDbGFzczsAHkxkYWx2aWsvYW5ub3RhdGlvbi9Jbm5lckNsYXNzOwASTGph"
+ + "dmEvbGFuZy9PYmplY3Q7ABJMamF2YS9sYW5nL1N0cmluZzsAGUxqYXZhL2xhbmcvU3RyaW5nQnVp"
+ + "bGRlcjsAEkxqYXZhL2xhbmcvVGhyZWFkOwANVGVzdDIwMDUuamF2YQAJVHJhbnNmb3JtAAFWAAth"
+ + "Y2Nlc3NGbGFncwAGYXBwZW5kAA1jdXJyZW50VGhyZWFkAAdnZXROYW1lAA5ncmVldGluZ0Rhbmlz"
+ + "aAAPZ3JlZXRpbmdFbmdsaXNoAA5ncmVldGluZ0ZyZW5jaAAQZ3JlZXRpbmdKYXBhbmVzZQAEbmFt"
+ + "ZQAFc2F5SGkAC3NheUhpRGFuaXNoAAxzYXlIaUVuZ2xpc2gAC3NheUhpRnJlbmNoAA1zYXlIaUph"
+ + "cGFuZXNlAAh0b1N0cmluZwAFdmFsdWUAiwF+fkQ4eyJjb21waWxhdGlvbi1tb2RlIjoiZGVidWci"
+ + "LCJoYXMtY2hlY2tzdW1zIjpmYWxzZSwibWluLWFwaSI6MSwic2hhLTEiOiI5N2RmNmVkNzlhNzQw"
+ + "ZWVhMzM4MmNiNWRhOTIyYmI1YmJjMDg2NDMzIiwidmVyc2lvbiI6IjIuMC45LWRldiJ9AAfjgZPj"
+ + "gpPjgavjgaHjga/kuJbnlYwAAgIBIhgBAgMCEwQZGxcRAAQBBQABAQEBAQEBAIGABOwFAQH4AwEB"
+ + "jAUBAaQFAQG8BQEB1AUAAAAAAAAAAgAAANwFAADiBQAAGAYAAAAAAAAAAAAAAAAAABAAAAAAAAAA"
+ + "AQAAAAAAAAABAAAAJQAAAHAAAAACAAAACQAAAAQBAAADAAAABAAAACgBAAAEAAAABAAAAFgBAAAF"
+ + "AAAADAAAAHgBAAAGAAAAAQAAANgBAAABIAAABgAAAPgBAAADIAAABgAAACQDAAABEAAAAQAAAEQD"
+ + "AAACIAAAJQAAAEoDAAAEIAAAAgAAANwFAAAAIAAAAQAAAOsFAAADEAAAAgAAABQGAAAGIAAAAQAA"
+ + "ACQGAAAAEAAAAQAAADQGAAA=");
+
+ public static void run() throws Exception {
+ Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE);
+ doTest();
+ }
+
+ public static final class MyThread extends Thread {
+ public MyThread(CountDownLatch delay, int id) {
+ super("Thread: " + id);
+ this.thr_id = id;
+ this.results = new ArrayList<>(1000);
+ this.finish = false;
+ this.delay = delay;
+ }
+
+ public void run() {
+ delay.countDown();
+ while (!finish) {
+ Transform t = new Transform();
+ results.add(t.sayHi());
+ }
+ }
+
+ public void finish() throws Exception {
+ finish = true;
+ this.join();
+ }
+
+ public void Check() throws Exception {
+ for (String s : results) {
+ if (!s.equals("Hello from " + getName())
+ && !s.equals("Hello, " + DEFAULT_VAL + ", " + DEFAULT_VAL + ", " + DEFAULT_VAL
+ + " from " + getName())
+ && !s.equals(
+ "Hello World, Bonjour le Monde, Hej Verden, こんにちは世界 from " + getName())) {
+ System.out.println("FAIL " + thr_id + ": Unexpected result: " + s);
+ }
+ }
+ }
+
+ public ArrayList<String> results;
+ public volatile boolean finish;
+ public int thr_id;
+ public CountDownLatch delay;
+ }
+
+ public static MyThread[] startThreads(int num_threads) throws Exception {
+ CountDownLatch cdl = new CountDownLatch(num_threads);
+ MyThread[] res = new MyThread[num_threads];
+ for (int i = 0; i < num_threads; i++) {
+ res[i] = new MyThread(cdl, i);
+ res[i].start();
+ }
+ cdl.await();
+ return res;
+ }
+ public static void finishThreads(MyThread[] thrs) throws Exception {
+ for (MyThread t : thrs) {
+ t.finish();
+ }
+ for (MyThread t : thrs) {
+ t.Check();
+ }
+ }
+
+ public static void doRedefinition() throws Exception {
+ // Get the current set of fields.
+ Field[] fields = Transform.class.getDeclaredFields();
+ // Get all the threads in the 'main' thread group
+ ThreadGroup mytg = Thread.currentThread().getThreadGroup();
+ Thread[] all_threads = new Thread[mytg.activeCount()];
+ mytg.enumerate(all_threads);
+ Set<Thread> thread_set = new HashSet<>(Arrays.asList(all_threads));
+ thread_set.remove(Thread.currentThread());
+ // Suspend them.
+ Suspension.suspendList(thread_set.toArray(new Thread[0]));
+ // Actual redefine.
+ Redefinition.doCommonStructuralClassRedefinition(Transform.class, DEX_BYTES);
+ // Get the new fields.
+ Field[] new_fields = Transform.class.getDeclaredFields();
+ Set<Field> field_set = new HashSet(Arrays.asList(new_fields));
+ field_set.removeAll(Arrays.asList(fields));
+ // Initialize the new fields on the old objects and resume.
+ UpdateFieldValuesAndResumeThreads(thread_set.toArray(new Thread[0]),
+ Transform.class,
+ field_set.toArray(new Field[0]),
+ DEFAULT_VAL);
+ }
+
+ public static void doTest() throws Exception {
+ MyThread[] threads = startThreads(NUM_THREADS);
+
+ doRedefinition();
+ finishThreads(threads);
+ }
+ public static native void UpdateFieldValuesAndResumeThreads(
+ Thread[] t, Class<?> redefined_class, Field[] new_fields, String default_val);
+}
diff --git a/test/925-threadgroups/src/art/Test925.java b/test/925-threadgroups/src/art/Test925.java
index 0779f63..a63f4ce 100644
--- a/test/925-threadgroups/src/art/Test925.java
+++ b/test/925-threadgroups/src/art/Test925.java
@@ -124,7 +124,16 @@
for (int i = 0; i < timeoutS; i++) {
Object[] data = getThreadGroupChildren(tg);
Thread[] threads = (Thread[])data[0];
- if (threads.length == expectedChildCount) {
+ List<Thread> lthreads = new ArrayList<>(Arrays.asList(threads));
+ Iterator<Thread> it = lthreads.iterator();
+ while (it.hasNext()) {
+ Thread t = it.next();
+ if (t.getName().startsWith("Jit thread pool worker")) {
+ it.remove();
+ break;
+ }
+ }
+ if (lthreads.size() == expectedChildCount) {
return;
}
Thread.sleep(1000);
diff --git a/test/Android.bp b/test/Android.bp
index de2ab3f..0051853 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -361,6 +361,7 @@
"1974-resize-array/resize_array.cc",
"1975-hello-structural-transformation/structural_transform.cc",
"1976-hello-structural-static-methods/structural_transform_methods.cc",
+ "2005-pause-all-redefine-multithreaded/pause-all.cc",
],
// Use NDK-compatible headers for ctstiagent.
header_libs: [
