Fix GetThreadState with threads in kNative.
We were always returning JVMTI_THREAD_STATE_WAITING for threads in the
kNative state. To prevent any similar problems from happening in the
future we changed it so all thread-states are explicitly enumerated
and handled in a switch statement.
Test: ./test.py --host -j50
Bug: 67784165
Change-Id: I6646b36aa36cb4671bf95777aefc5c88b659e90f
diff --git a/openjdkjvmti/ti_thread.cc b/openjdkjvmti/ti_thread.cc
index 9a809df..d0913cf 100644
--- a/openjdkjvmti/ti_thread.cc
+++ b/openjdkjvmti/ti_thread.cc
@@ -340,47 +340,92 @@
jint jvmti_state = JVMTI_THREAD_STATE_ALIVE;
if (state.thread_user_code_suspend_count != 0) {
+ // Suspended can be set with any thread state so check it here. Even if the thread isn't in
+ // kSuspended state it will move to that once it hits a checkpoint so we can still set this.
jvmti_state |= JVMTI_THREAD_STATE_SUSPENDED;
// Note: We do not have data about the previous state. Otherwise we should load the previous
// state here.
}
if (state.native_thread->IsInterrupted()) {
+ // Interrupted can be set with any thread state so check it here.
jvmti_state |= JVMTI_THREAD_STATE_INTERRUPTED;
}
- if (internal_thread_state == art::ThreadState::kNative) {
- jvmti_state |= JVMTI_THREAD_STATE_IN_NATIVE;
+ // Enumerate all the thread states and fill in the other bits. This contains the results of
+ // following the decision tree in the JVMTI spec GetThreadState documentation.
+ switch (internal_thread_state) {
+ case art::ThreadState::kRunnable:
+ case art::ThreadState::kWaitingWeakGcRootRead:
+ case art::ThreadState::kSuspended:
+ // These are all simply runnable.
+ // kRunnable is self-explanatory.
+ // kWaitingWeakGcRootRead is set during some operations with strings due to the intern-table
+ // so we want to keep it marked as runnable.
+ // kSuspended we don't mark since if we don't have a user_code_suspend_count then it is done
+ // by the GC and not a JVMTI suspension, which means it cannot be removed by ResumeThread.
+ jvmti_state |= JVMTI_THREAD_STATE_RUNNABLE;
+ break;
+ case art::ThreadState::kNative:
+ // kNative means native and runnable. Technically THREAD_STATE_IN_NATIVE can be set with any
+ // state but we don't have the information to know if it should be present for any but the
+ // kNative state.
+ jvmti_state |= (JVMTI_THREAD_STATE_IN_NATIVE |
+ JVMTI_THREAD_STATE_RUNNABLE);
+ break;
+ case art::ThreadState::kBlocked:
+ // Blocked is one of the top level states so it sits alone.
+ jvmti_state |= JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER;
+ break;
+ case art::ThreadState::kWaiting:
+ // Object.wait() so waiting, indefinitely, in object.wait.
+ jvmti_state |= (JVMTI_THREAD_STATE_WAITING |
+ JVMTI_THREAD_STATE_WAITING_INDEFINITELY |
+ JVMTI_THREAD_STATE_IN_OBJECT_WAIT);
+ break;
+ case art::ThreadState::kTimedWaiting:
+ // Object.wait(long) so waiting, with timeout, in object.wait.
+ jvmti_state |= (JVMTI_THREAD_STATE_WAITING |
+ JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT |
+ JVMTI_THREAD_STATE_IN_OBJECT_WAIT);
+ break;
+ case art::ThreadState::kSleeping:
+ // In object.sleep. This is a timed wait caused by sleep.
+ jvmti_state |= (JVMTI_THREAD_STATE_WAITING |
+ JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT |
+ JVMTI_THREAD_STATE_SLEEPING);
+ break;
+ // TODO We might want to print warnings if we have the debugger running while JVMTI agents are
+ // attached.
+ case art::ThreadState::kWaitingForDebuggerSend:
+ case art::ThreadState::kWaitingForDebuggerToAttach:
+ case art::ThreadState::kWaitingInMainDebuggerLoop:
+ case art::ThreadState::kWaitingForDebuggerSuspension:
+ case art::ThreadState::kWaitingForLockInflation:
+ case art::ThreadState::kWaitingForTaskProcessor:
+ case art::ThreadState::kWaitingForGcToComplete:
+ case art::ThreadState::kWaitingForCheckPointsToRun:
+ case art::ThreadState::kWaitingPerformingGc:
+ case art::ThreadState::kWaitingForJniOnLoad:
+ case art::ThreadState::kWaitingInMainSignalCatcherLoop:
+ case art::ThreadState::kWaitingForSignalCatcherOutput:
+ case art::ThreadState::kWaitingForDeoptimization:
+ case art::ThreadState::kWaitingForMethodTracingStart:
+ case art::ThreadState::kWaitingForVisitObjects:
+ case art::ThreadState::kWaitingForGetObjectsAllocated:
+ case art::ThreadState::kWaitingForGcThreadFlip:
+ // All of these are causing the thread to wait for an indeterminate amount of time but isn't
+ // caused by sleep, park, or object#wait.
+ jvmti_state |= (JVMTI_THREAD_STATE_WAITING |
+ JVMTI_THREAD_STATE_WAITING_INDEFINITELY);
+ break;
+ case art::ThreadState::kStarting:
+ case art::ThreadState::kTerminated:
+ // We only call this if we are alive so we shouldn't see either of these states.
+ LOG(FATAL) << "Should not be in state " << internal_thread_state;
+ UNREACHABLE();
}
-
- if (internal_thread_state == art::ThreadState::kRunnable ||
- internal_thread_state == art::ThreadState::kWaitingWeakGcRootRead ||
- internal_thread_state == art::ThreadState::kSuspended) {
- jvmti_state |= JVMTI_THREAD_STATE_RUNNABLE;
- } else if (internal_thread_state == art::ThreadState::kBlocked) {
- jvmti_state |= JVMTI_THREAD_STATE_BLOCKED_ON_MONITOR_ENTER;
- } else {
- // Should be in waiting state.
- jvmti_state |= JVMTI_THREAD_STATE_WAITING;
-
- if (internal_thread_state == art::ThreadState::kTimedWaiting ||
- internal_thread_state == art::ThreadState::kSleeping) {
- jvmti_state |= JVMTI_THREAD_STATE_WAITING_WITH_TIMEOUT;
- } else {
- jvmti_state |= JVMTI_THREAD_STATE_WAITING_INDEFINITELY;
- }
-
- if (internal_thread_state == art::ThreadState::kSleeping) {
- jvmti_state |= JVMTI_THREAD_STATE_SLEEPING;
- }
-
- if (internal_thread_state == art::ThreadState::kTimedWaiting ||
- internal_thread_state == art::ThreadState::kWaiting) {
- jvmti_state |= JVMTI_THREAD_STATE_IN_OBJECT_WAIT;
- }
-
- // TODO: PARKED. We'll have to inspect the stack.
- }
+ // TODO: PARKED. We'll have to inspect the stack.
return jvmti_state;
}
diff --git a/test/924-threads/expected.txt b/test/924-threads/expected.txt
index e529559..accc782 100644
--- a/test/924-threads/expected.txt
+++ b/test/924-threads/expected.txt
@@ -33,6 +33,7 @@
401 = ALIVE|BLOCKED_ON_MONITOR_ENTER
e1 = ALIVE|WAITING_WITH_TIMEOUT|SLEEPING|WAITING
5 = ALIVE|RUNNABLE
+400005 = ALIVE|RUNNABLE|IN_NATIVE
2 = TERMINATED
Thread type is class art.Test924$ExtThread
0 = NEW
@@ -41,6 +42,7 @@
401 = ALIVE|BLOCKED_ON_MONITOR_ENTER
e1 = ALIVE|WAITING_WITH_TIMEOUT|SLEEPING|WAITING
5 = ALIVE|RUNNABLE
+400005 = ALIVE|RUNNABLE|IN_NATIVE
2 = TERMINATED
[Thread[FinalizerDaemon,5,system], Thread[FinalizerWatchdogDaemon,5,system], Thread[HeapTaskDaemon,5,system], Thread[ReferenceQueueDaemon,5,system], Thread[TestThread,5,main], Thread[main,5,main]]
JVMTI_ERROR_THREAD_NOT_ALIVE
diff --git a/test/924-threads/src/art/Test924.java b/test/924-threads/src/art/Test924.java
index 1ff2c3f..e8e9781 100644
--- a/test/924-threads/src/art/Test924.java
+++ b/test/924-threads/src/art/Test924.java
@@ -109,6 +109,7 @@
final CountDownLatch cdl4 = new CountDownLatch(1);
final CountDownLatch cdl5 = new CountDownLatch(1);
final Holder h = new Holder();
+ final NativeWaiter w = new NativeWaiter();
Runnable r = new Runnable() {
@Override
public void run() {
@@ -136,6 +137,8 @@
while (!h.flag) {
// Busy-loop.
}
+
+ nativeLoop(w.struct);
} catch (Exception e) {
throw new RuntimeException(e);
}
@@ -193,6 +196,11 @@
printThreadState(t);
h.flag = true;
+ // Native
+ w.waitForNative();
+ printThreadState(t);
+ w.finish();
+
// Dying.
t.join();
Thread.yield();
@@ -427,6 +435,31 @@
System.out.println(threadInfo[4] == null ? "null" : threadInfo[4].getClass()); // Context CL.
}
+ public static final class NativeWaiter {
+ public long struct;
+ public NativeWaiter() {
+ struct = nativeWaiterStructAlloc();
+ }
+ public void waitForNative() {
+ if (struct == 0l) {
+ throw new Error("Already resumed from native!");
+ }
+ nativeWaiterStructWaitForNative(struct);
+ }
+ public void finish() {
+ if (struct == 0l) {
+ throw new Error("Already resumed from native!");
+ }
+ nativeWaiterStructFinish(struct);
+ struct = 0;
+ }
+ }
+
+ private static native long nativeWaiterStructAlloc();
+ private static native void nativeWaiterStructWaitForNative(long struct);
+ private static native void nativeWaiterStructFinish(long struct);
+ private static native void nativeLoop(long w);
+
private static native Thread getCurrentThread();
private static native Object[] getThreadInfo(Thread t);
private static native int getThreadState(Thread t);
diff --git a/test/924-threads/threads.cc b/test/924-threads/threads.cc
index e21dcc2..8caff76 100644
--- a/test/924-threads/threads.cc
+++ b/test/924-threads/threads.cc
@@ -35,6 +35,46 @@
namespace art {
namespace Test924Threads {
+struct WaiterStruct {
+ std::atomic<bool> started;
+ std::atomic<bool> finish;
+};
+
+extern "C" JNIEXPORT jlong JNICALL Java_art_Test924_nativeWaiterStructAlloc(
+ JNIEnv* env, jclass TestClass ATTRIBUTE_UNUSED) {
+ WaiterStruct* s = nullptr;
+ if (JvmtiErrorToException(env,
+ jvmti_env,
+ jvmti_env->Allocate(sizeof(WaiterStruct),
+ reinterpret_cast<unsigned char**>(&s)))) {
+ return 0;
+ }
+ s->started = false;
+ s->finish = false;
+ return static_cast<jlong>(reinterpret_cast<intptr_t>(s));
+}
+
+extern "C" JNIEXPORT void JNICALL Java_art_Test924_nativeWaiterStructWaitForNative(
+ JNIEnv* env ATTRIBUTE_UNUSED, jclass TestClass ATTRIBUTE_UNUSED, jlong waiter_struct) {
+ WaiterStruct* s = reinterpret_cast<WaiterStruct*>(static_cast<intptr_t>(waiter_struct));
+ while (!s->started) { }
+}
+
+extern "C" JNIEXPORT void JNICALL Java_art_Test924_nativeWaiterStructFinish(
+ JNIEnv* env ATTRIBUTE_UNUSED, jclass TestClass ATTRIBUTE_UNUSED, jlong waiter_struct) {
+ WaiterStruct* s = reinterpret_cast<WaiterStruct*>(static_cast<intptr_t>(waiter_struct));
+ s->finish = true;
+}
+
+extern "C" JNIEXPORT void JNICALL Java_art_Test924_nativeLoop(JNIEnv* env,
+ jclass TestClass ATTRIBUTE_UNUSED,
+ jlong waiter_struct) {
+ WaiterStruct* s = reinterpret_cast<WaiterStruct*>(static_cast<intptr_t>(waiter_struct));
+ s->started = true;
+ while (!s->finish) { }
+ JvmtiErrorToException(env, jvmti_env, jvmti_env->Deallocate(reinterpret_cast<unsigned char*>(s)));
+}
+
// private static native Thread getCurrentThread();
// private static native Object[] getThreadInfo(Thread t);