Do not hold lock when making class visibly initialized.
Doing so can lead to deadlocks.
Bug: 138561860
Bug: 36692143
Test: New test 177-visibly-initialized-deadlock
Test: m test-art-host-gtest
Test: testrunner.py --host --optimizing
Test: aosp_taimen-userdebug boots.
Test: run-gtests.sh
Test: testrunner.py --target --optimizing
Change-Id: I6195a4a5a7d865f90c529da684697b9a3e23ff30
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 33016a2..9d9abe3 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -302,7 +302,6 @@
klass.Assign(ObjPtr<mirror::Class>::DownCast(self->DecodeJObject(classes_[i])));
vm->DeleteWeakGlobalRef(self, classes_[i]);
if (klass != nullptr) {
- ObjectLock<mirror::Class> lock(self, klass);
mirror::Class::SetStatus(klass, ClassStatus::kVisiblyInitialized, self);
}
}
diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc
index dde056a..7dff9df 100644
--- a/runtime/mirror/class.cc
+++ b/runtime/mirror/class.cc
@@ -154,7 +154,11 @@
LOG(FATAL) << "Unexpected change back of class status for " << h_this->PrettyClass()
<< " " << old_status << " -> " << new_status;
}
- if (new_status >= ClassStatus::kResolved || old_status >= ClassStatus::kResolved) {
+ if (old_status == ClassStatus::kInitialized) {
+ // We do not hold the lock for making the class visibly initialized
+ // as this is unnecessary and could lead to deadlocks.
+ CHECK_EQ(new_status, ClassStatus::kVisiblyInitialized);
+ } else if (new_status >= ClassStatus::kResolved || old_status >= ClassStatus::kResolved) {
// When classes are being resolved the resolution code should hold the lock.
CHECK_EQ(h_this->GetLockOwnerThreadId(), self->GetThreadId())
<< "Attempt to change status of class while not holding its lock: "
@@ -228,6 +232,10 @@
if (new_status == ClassStatus::kRetired || new_status == ClassStatus::kErrorUnresolved) {
h_this->NotifyAll(self);
}
+ } else if (old_status == ClassStatus::kInitialized) {
+ // Do not notify for transition from kInitialized to ClassStatus::kVisiblyInitialized.
+ // This is a hidden transition, not observable by bytecode.
+ DCHECK_EQ(new_status, ClassStatus::kVisiblyInitialized); // Already CHECK()ed above.
} else {
CHECK_NE(new_status, ClassStatus::kRetired);
if (old_status >= ClassStatus::kResolved || new_status >= ClassStatus::kResolved) {
diff --git a/test/177-visibly-initialized-deadlock/expected.txt b/test/177-visibly-initialized-deadlock/expected.txt
new file mode 100644
index 0000000..6a5618e
--- /dev/null
+++ b/test/177-visibly-initialized-deadlock/expected.txt
@@ -0,0 +1 @@
+JNI_OnLoad called
diff --git a/test/177-visibly-initialized-deadlock/info.txt b/test/177-visibly-initialized-deadlock/info.txt
new file mode 100644
index 0000000..9ede3d5
--- /dev/null
+++ b/test/177-visibly-initialized-deadlock/info.txt
@@ -0,0 +1,2 @@
+Regression test for deadlock when trying to make class visibly initialized.
+b/138561860
diff --git a/test/177-visibly-initialized-deadlock/src/Main.java b/test/177-visibly-initialized-deadlock/src/Main.java
new file mode 100644
index 0000000..1755f54
--- /dev/null
+++ b/test/177-visibly-initialized-deadlock/src/Main.java
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2019 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.
+ */
+
+class Main {
+ private static final boolean DEBUG = false;
+
+ public static void main(String[] args) throws Exception {
+ System.loadLibrary(args[0]);
+ makeVisiblyInitialized();
+ Class<?> testClass = Class.forName("TestClass"); // Request initialized class.
+ boolean is_visibly_initialized = isVisiblyInitialized(testClass);
+ if (DEBUG) {
+ System.out.println((is_visibly_initialized ? "Already" : "Not yet") + " visibly intialized");
+ }
+ if (!is_visibly_initialized) {
+ synchronized(testClass) {
+ Thread t = new Thread() {
+ public void run() {
+ // Regression test: This would have previously deadlocked
+ // trying to lock on testClass. b/138561860
+ makeVisiblyInitialized();
+ }
+ };
+ t.start();
+ t.join();
+ }
+ if (!isVisiblyInitialized(testClass)) {
+ throw new Error("Should be visibly initialized now.");
+ }
+ }
+ }
+
+ public static native void makeVisiblyInitialized();
+ public static native boolean isVisiblyInitialized(Class<?> klass);
+}
+
+class TestClass {
+ static {
+ // Add a static constructor that prevents initialization at compile time (app images).
+ Main.isVisiblyInitialized(TestClass.class); // Native call, discard result.
+ }
+}
diff --git a/test/177-visibly-initialized-deadlock/visibly_initialized.cc b/test/177-visibly-initialized-deadlock/visibly_initialized.cc
new file mode 100644
index 0000000..44bb318
--- /dev/null
+++ b/test/177-visibly-initialized-deadlock/visibly_initialized.cc
@@ -0,0 +1,38 @@
+/*
+ * Copyright 2019 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 "class_linker.h"
+#include "runtime.h"
+#include "scoped_thread_state_change-inl.h"
+#include "thread-current-inl.h"
+
+namespace art {
+
+extern "C" JNIEXPORT void JNICALL Java_Main_makeVisiblyInitialized(JNIEnv*, jclass) {
+ Runtime::Current()->GetClassLinker()->MakeInitializedClassesVisiblyInitialized(
+ Thread::Current(), /*wait=*/ true);
+}
+
+extern "C" JNIEXPORT jboolean JNICALL Java_Main_isVisiblyInitialized(JNIEnv*, jclass, jclass c) {
+ ScopedObjectAccess soa(Thread::Current());
+ ObjPtr<mirror::Class> klass = soa.Decode<mirror::Class>(c);
+ return klass->IsVisiblyInitialized() ? JNI_TRUE : JNI_FALSE;
+}
+
+} // namespace art
diff --git a/test/Android.bp b/test/Android.bp
index d1382ab..baf5e22 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -478,6 +478,7 @@
"167-visit-locks/visit_locks.cc",
"169-threadgroup-jni/jni_daemon_thread.cc",
"172-app-image-twice/debug_print_class.cc",
+ "177-visibly-initialized-deadlock/visibly_initialized.cc",
"1945-proxy-method-arguments/get_args.cc",
"203-multi-checkpoint/multi_checkpoint.cc",
"305-other-fault-handler/fault_handler.cc",
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 85eec1b..2a64ba6 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -874,6 +874,7 @@
"167-visit-locks",
"168-vmstack-annotated",
"172-app-image-twice",
+ "177-visibly-initialized-deadlock",
"201-built-in-except-detail-messages",
"203-multi-checkpoint",
"304-method-tracing",