Revert "Correctly handle thread deopt with thread-specific JVMTI events"
This reverts commit c8c7e0659a76a951f3ab110ed9b7f37777448d36.
Test: none.
Reason for revert: Causes art-tracing to flake/fail pop-frame tests.
Bug: 131865028
Change-Id: I16bda461da94405f372b6b0be54ac4b4efb08fda
diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index 40e8b80..16abf41 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -974,8 +974,6 @@
}
enum class DeoptRequirement {
- // No deoptimization work required.
- kNone,
// Limited/no deopt required.
kLimited,
// A single thread must be put into interpret only.
@@ -1000,38 +998,19 @@
case ArtJvmtiEvent::kSingleStep:
case ArtJvmtiEvent::kFramePop:
return thread == nullptr ? DeoptRequirement::kFull : DeoptRequirement::kThread;
- case ArtJvmtiEvent::kVmInit:
- case ArtJvmtiEvent::kVmDeath:
- case ArtJvmtiEvent::kThreadStart:
- case ArtJvmtiEvent::kThreadEnd:
- case ArtJvmtiEvent::kClassFileLoadHookNonRetransformable:
- case ArtJvmtiEvent::kClassLoad:
- case ArtJvmtiEvent::kClassPrepare:
- case ArtJvmtiEvent::kVmStart:
- case ArtJvmtiEvent::kNativeMethodBind:
- case ArtJvmtiEvent::kCompiledMethodLoad:
- case ArtJvmtiEvent::kCompiledMethodUnload:
- case ArtJvmtiEvent::kDynamicCodeGenerated:
- case ArtJvmtiEvent::kDataDumpRequest:
- case ArtJvmtiEvent::kMonitorWait:
- case ArtJvmtiEvent::kMonitorWaited:
- case ArtJvmtiEvent::kMonitorContendedEnter:
- case ArtJvmtiEvent::kMonitorContendedEntered:
- case ArtJvmtiEvent::kResourceExhausted:
- case ArtJvmtiEvent::kGarbageCollectionStart:
- case ArtJvmtiEvent::kGarbageCollectionFinish:
- case ArtJvmtiEvent::kObjectFree:
- case ArtJvmtiEvent::kVmObjectAlloc:
- case ArtJvmtiEvent::kClassFileLoadHookRetransformable:
- case ArtJvmtiEvent::kDdmPublishChunk:
- return DeoptRequirement::kNone;
+ default:
+ LOG(FATAL) << "Unexpected event type!";
+ UNREACHABLE();
}
}
-jvmtiError EventHandler::HandleEventDeopt(ArtJvmtiEvent event, jthread thread, bool enable) {
+jvmtiError EventHandler::SetupTraceListener(JvmtiMethodTraceListener* listener,
+ ArtJvmtiEvent event,
+ jthread thread,
+ bool enable) {
DeoptRequirement deopt_req = GetDeoptRequirement(event, thread);
// Make sure we can deopt.
- if (deopt_req != DeoptRequirement::kNone) {
+ {
art::ScopedObjectAccess soa(art::Thread::Current());
DeoptManager* deopt_manager = DeoptManager::Get();
jvmtiError err = OK;
@@ -1068,12 +1047,7 @@
}
}
}
- return OK;
-}
-void EventHandler::SetupTraceListener(JvmtiMethodTraceListener* listener,
- ArtJvmtiEvent event,
- bool enable) {
// Add the actual listeners.
uint32_t new_events = GetInstrumentationEventsFor(event);
if (new_events == art::instrumentation::Instrumentation::kDexPcMoved) {
@@ -1086,7 +1060,7 @@
if (IsEventEnabledAnywhere(other)) {
// The event needs to be kept around/is already enabled by the other jvmti event that uses the
// same instrumentation event.
- return;
+ return OK;
}
}
art::ScopedThreadStateChange stsc(art::Thread::Current(), art::ThreadState::kNative);
@@ -1097,7 +1071,7 @@
} else {
instr->RemoveListener(listener, new_events);
}
- return;
+ return OK;
}
// Makes sure that all compiled methods are AsyncDeoptimizable so we can deoptimize (and force to
@@ -1153,10 +1127,11 @@
return false;
}
-void EventHandler::SetupFramePopTraceListener(bool enable) {
+jvmtiError EventHandler::SetupFramePopTraceListener(jthread thread, bool enable) {
if (enable) {
frame_pop_enabled = true;
- SetupTraceListener(method_trace_listener_.get(), ArtJvmtiEvent::kFramePop, enable);
+ return SetupTraceListener(
+ method_trace_listener_.get(), ArtJvmtiEvent::kFramePop, thread, enable);
} else {
// remove the listener if we have no outstanding frames.
{
@@ -1165,37 +1140,38 @@
art::ReaderMutexLock event_mu(art::Thread::Current(), env->event_info_mutex_);
if (!env->notify_frames.empty()) {
// Leaving FramePop listener since there are unsent FramePop events.
- return;
+ return OK;
}
}
frame_pop_enabled = false;
}
- SetupTraceListener(method_trace_listener_.get(), ArtJvmtiEvent::kFramePop, enable);
+ return SetupTraceListener(
+ method_trace_listener_.get(), ArtJvmtiEvent::kFramePop, thread, enable);
}
}
// Handle special work for the given event type, if necessary.
-void EventHandler::HandleEventType(ArtJvmtiEvent event, bool enable) {
+jvmtiError EventHandler::HandleEventType(ArtJvmtiEvent event, jthread thread, bool enable) {
switch (event) {
case ArtJvmtiEvent::kDdmPublishChunk:
SetupDdmTracking(ddm_listener_.get(), enable);
- return;
+ return OK;
case ArtJvmtiEvent::kVmObjectAlloc:
SetupObjectAllocationTracking(alloc_listener_.get(), enable);
- return;
+ return OK;
case ArtJvmtiEvent::kGarbageCollectionStart:
case ArtJvmtiEvent::kGarbageCollectionFinish:
SetupGcPauseTracking(gc_pause_listener_.get(), event, enable);
- return;
+ return OK;
// FramePop can never be disabled once it's been turned on if it was turned off with outstanding
// pop-events since we would either need to deal with dangling pointers or have missed events.
case ArtJvmtiEvent::kFramePop:
if (enable && frame_pop_enabled) {
// The frame-pop event was held on by pending events so we don't need to do anything.
+ break;
} else {
- SetupFramePopTraceListener(enable);
+ return SetupFramePopTraceListener(thread, enable);
}
- return;
case ArtJvmtiEvent::kMethodEntry:
case ArtJvmtiEvent::kMethodExit:
case ArtJvmtiEvent::kFieldAccess:
@@ -1204,8 +1180,7 @@
case ArtJvmtiEvent::kExceptionCatch:
case ArtJvmtiEvent::kBreakpoint:
case ArtJvmtiEvent::kSingleStep:
- SetupTraceListener(method_trace_listener_.get(), event, enable);
- return;
+ return SetupTraceListener(method_trace_listener_.get(), event, thread, enable);
case ArtJvmtiEvent::kMonitorContendedEnter:
case ArtJvmtiEvent::kMonitorContendedEntered:
case ArtJvmtiEvent::kMonitorWait:
@@ -1213,11 +1188,11 @@
if (!OtherMonitorEventsEnabledAnywhere(event)) {
SetupMonitorListener(monitor_listener_.get(), park_listener_.get(), enable);
}
- return;
+ return OK;
default:
break;
}
- return;
+ return OK;
}
// Checks to see if the env has the capabilities associated with the given event.
@@ -1301,15 +1276,8 @@
art::Thread* self = art::Thread::Current();
art::Thread* target = nullptr;
ScopedNoUserCodeSuspension snucs(self);
- // The overall state across all threads and jvmtiEnvs. This is used to control the state of the
- // instrumentation handlers since we only want each added once.
bool old_state;
bool new_state;
- // The state for just the current 'thread' (including null) across all jvmtiEnvs. This is used to
- // control the deoptimization state since we do refcounting for that and need to perform different
- // actions depending on if the event is limited to a single thread or global.
- bool old_thread_state;
- bool new_thread_state;
{
// From now on we know we cannot get suspended by user-code.
// NB This does a SuspendCheck (during thread state change) so we need to
@@ -1328,55 +1296,28 @@
}
}
-
art::WriterMutexLock ei_mu(self, env->event_info_mutex_);
- old_thread_state = GetThreadEventState(event, target);
old_state = global_mask.Test(event);
if (mode == JVMTI_ENABLE) {
env->event_masks.EnableEvent(env, target, event);
global_mask.Set(event);
new_state = true;
- new_thread_state = true;
- DCHECK(GetThreadEventState(event, target));
} else {
DCHECK_EQ(mode, JVMTI_DISABLE);
env->event_masks.DisableEvent(env, target, event);
RecalculateGlobalEventMaskLocked(event);
new_state = global_mask.Test(event);
- new_thread_state = GetThreadEventState(event, target);
- DCHECK(new_state || !new_thread_state);
}
}
// Handle any special work required for the event type. We still have the
// user_code_suspend_count_lock_ so there won't be any interleaving here.
if (new_state != old_state) {
- HandleEventType(event, mode == JVMTI_ENABLE);
- }
- if (old_thread_state != new_thread_state) {
- return HandleEventDeopt(event, thread, new_thread_state);
+ return HandleEventType(event, thread, mode == JVMTI_ENABLE);
}
return OK;
}
-bool EventHandler::GetThreadEventState(ArtJvmtiEvent event, art::Thread* thread) {
- for (ArtJvmTiEnv* stored_env : envs) {
- if (stored_env == nullptr) {
- continue;
- }
- auto& masks = stored_env->event_masks;
- if (thread == nullptr && masks.global_event_mask.Test(event)) {
- return true;
- } else if (thread != nullptr) {
- EventMask* mask = masks.GetEventMaskOrNull(thread);
- if (mask != nullptr && mask->Test(event)) {
- return true;
- }
- }
- }
- return false;
-}
-
void EventHandler::HandleBreakpointEventsChanged(bool added) {
if (added) {
DeoptManager::Get()->AddDeoptimizationRequester();
diff --git a/openjdkjvmti/events.h b/openjdkjvmti/events.h
index d54c87a..2c3c7a0 100644
--- a/openjdkjvmti/events.h
+++ b/openjdkjvmti/events.h
@@ -247,10 +247,13 @@
REQUIRES(!envs_lock_);
private:
- void SetupTraceListener(JvmtiMethodTraceListener* listener, ArtJvmtiEvent event, bool enable);
+ jvmtiError SetupTraceListener(JvmtiMethodTraceListener* listener,
+ ArtJvmtiEvent event,
+ jthread thread,
+ bool enable);
// Specifically handle the FramePop event which it might not always be possible to turn off.
- void SetupFramePopTraceListener(bool enable);
+ jvmtiError SetupFramePopTraceListener(jthread thread, bool enable);
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
@@ -290,11 +293,6 @@
ALWAYS_INLINE
inline void RecalculateGlobalEventMaskLocked(ArtJvmtiEvent event) REQUIRES_SHARED(envs_lock_);
- // Returns whether there are any active requests for the given event on the given thread. This
- // should only be used while modifying the events for a thread.
- bool GetThreadEventState(ArtJvmtiEvent event, art::Thread* thread)
- REQUIRES(envs_lock_, art::Locks::thread_list_lock_);
-
template <ArtJvmtiEvent kEvent>
ALWAYS_INLINE inline void DispatchClassFileLoadHookEvent(art::Thread* thread,
JNIEnv* jnienv,
@@ -315,11 +313,7 @@
jclass klass) const
REQUIRES(!envs_lock_);
- // Sets up the global state needed for the first/last enable of an event across all threads
- void HandleEventType(ArtJvmtiEvent event, bool enable);
- // Perform deopts required for enabling the event on the given thread. Null thread indicates
- // global event enabled.
- jvmtiError HandleEventDeopt(ArtJvmtiEvent event, jthread thread, bool enable);
+ jvmtiError HandleEventType(ArtJvmtiEvent event, jthread thread, bool enable);
void HandleLocalAccessCapabilityAdded();
void HandleBreakpointEventsChanged(bool enable);
diff --git a/test/1962-multi-thread-events/expected.txt b/test/1962-multi-thread-events/expected.txt
deleted file mode 100644
index 841eb8b..0000000
--- a/test/1962-multi-thread-events/expected.txt
+++ /dev/null
@@ -1,4 +0,0 @@
-Events on thread 1:
- Hit event on T1 Thread
-Events on thread 2:
- Hit event on T2 Thread
diff --git a/test/1962-multi-thread-events/info.txt b/test/1962-multi-thread-events/info.txt
deleted file mode 100644
index 75a47b8..0000000
--- a/test/1962-multi-thread-events/info.txt
+++ /dev/null
@@ -1,5 +0,0 @@
-Tests b/131865028
-
-Due to a mistake in the single-thread deopt CL we would incorrectly only update a threads
-deoptimization count if it was the first thread of a particular event type to be activated. This
-could cause events to be missed.
diff --git a/test/1962-multi-thread-events/multi_thread_events.cc b/test/1962-multi-thread-events/multi_thread_events.cc
deleted file mode 100644
index aeb15b0..0000000
--- a/test/1962-multi-thread-events/multi_thread_events.cc
+++ /dev/null
@@ -1,91 +0,0 @@
-/*
- * Copyright (C) 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 <stdio.h>
-
-#include "android-base/macros.h"
-
-#include "jni.h"
-#include "jvmti.h"
-#include "scoped_local_ref.h"
-
-// Test infrastructure
-#include "jni_helper.h"
-#include "jvmti_helper.h"
-#include "test_env.h"
-
-namespace art {
-namespace Test1962MultiThreadEvents {
-
-struct BreakpointData {
- jobject events;
- jmethodID target;
-};
-void cbMethodEntry(jvmtiEnv* jvmti,
- JNIEnv* env,
- jthread thread,
- jmethodID method,
- jboolean was_exception ATTRIBUTE_UNUSED,
- jvalue val ATTRIBUTE_UNUSED) {
- BreakpointData* data = nullptr;
- if (JvmtiErrorToException(
- env, jvmti, jvmti->GetThreadLocalStorage(thread, reinterpret_cast<void**>(&data)))) {
- return;
- }
- if (data->target != method) {
- return;
- }
- jclass klass = env->FindClass("art/Test1962");
- jmethodID handler =
- env->GetStaticMethodID(klass, "HandleEvent", "(Ljava/lang/Thread;Ljava/util/List;)V");
- CHECK(data != nullptr);
- env->CallStaticVoidMethod(klass, handler, thread, data->events);
-}
-
-extern "C" JNIEXPORT void JNICALL Java_art_Test1962_setupTest(JNIEnv* env,
- jclass klass ATTRIBUTE_UNUSED) {
- jvmtiCapabilities caps{
- .can_generate_method_exit_events = 1,
- };
- if (JvmtiErrorToException(env, jvmti_env, jvmti_env->AddCapabilities(&caps))) {
- return;
- }
- jvmtiEventCallbacks cb{
- .MethodExit = cbMethodEntry,
- };
- JvmtiErrorToException(env, jvmti_env, jvmti_env->SetEventCallbacks(&cb, sizeof(cb)));
-}
-
-extern "C" JNIEXPORT void JNICALL Java_art_Test1962_setupThread(
- JNIEnv* env, jclass klass ATTRIBUTE_UNUSED, jthread thr, jobject events, jobject target) {
- BreakpointData* data = nullptr;
- if (JvmtiErrorToException(
- env, jvmti_env, jvmti_env->Allocate(sizeof(*data), reinterpret_cast<uint8_t**>(&data)))) {
- return;
- }
- data->events = env->NewGlobalRef(events);
- data->target = env->FromReflectedMethod(target);
- if (JvmtiErrorToException(env, jvmti_env, jvmti_env->SetThreadLocalStorage(thr, data))) {
- return;
- }
- JvmtiErrorToException(
- env,
- jvmti_env,
- jvmti_env->SetEventNotificationMode(JVMTI_ENABLE, JVMTI_EVENT_METHOD_EXIT, thr));
-}
-
-} // namespace Test1962MultiThreadEvents
-} // namespace art
diff --git a/test/1962-multi-thread-events/run b/test/1962-multi-thread-events/run
deleted file mode 100755
index c6e62ae..0000000
--- a/test/1962-multi-thread-events/run
+++ /dev/null
@@ -1,17 +0,0 @@
-#!/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
diff --git a/test/1962-multi-thread-events/src/Main.java b/test/1962-multi-thread-events/src/Main.java
deleted file mode 100644
index 7669e21..0000000
--- a/test/1962-multi-thread-events/src/Main.java
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 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.
- */
-
-public class Main {
- public static void main(String[] args) throws Exception {
- art.Test1962.run();
- }
-}
diff --git a/test/1962-multi-thread-events/src/art/Test1962.java b/test/1962-multi-thread-events/src/art/Test1962.java
deleted file mode 100644
index b5c2992..0000000
--- a/test/1962-multi-thread-events/src/art/Test1962.java
+++ /dev/null
@@ -1,83 +0,0 @@
-/*
- * Copyright (C) 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.
- */
-
-package art;
-
-import java.lang.reflect.Method;
-import java.lang.reflect.Proxy;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.concurrent.CountDownLatch;
-
-public class Test1962 {
- public static void doNothing() {
- // We set a breakpoint here!
- }
- public static void run() throws Exception {
- doTest();
- }
-
- public static void HandleEvent(Thread t, List<String> events) {
- events.add("Hit event on " + t.getName());
- }
-
- public static void doTest() throws Exception {
- setupTest();
-
- final int NUM_THREADS = 2;
- List<String> t1Events = new ArrayList<>();
- List<String> t2Events = new ArrayList<>();
- final CountDownLatch threadResumeLatch = new CountDownLatch(1);
- final CountDownLatch threadStartLatch = new CountDownLatch(NUM_THREADS);
- Runnable threadRun = () -> {
- try {
- threadStartLatch.countDown();
- threadResumeLatch.await();
- doNothing();
- } catch (Exception e) {
- throw new Error("Failed at something", e);
- }
- };
- Thread T1 = new Thread(threadRun, "T1 Thread");
- Thread T2 = new Thread(threadRun, "T2 Thread");
- T1.start();
- T2.start();
- // Wait for both threads to have started.
- threadStartLatch.await();
- // Tell the threads to notify us when the doNothing method exits
- Method target = Test1962.class.getDeclaredMethod("doNothing");
- setupThread(T1, t1Events, target);
- setupThread(T2, t2Events, target);
- // Let the threads go.
- threadResumeLatch.countDown();
- // Wait for the threads to finish.
- T1.join();
- T2.join();
- // Print out the events each of the threads performed.
- System.out.println("Events on thread 1:");
- for (String s : t1Events) {
- System.out.println("\t" + s);
- }
- System.out.println("Events on thread 2:");
- for (String s : t2Events) {
- System.out.println("\t" + s);
- }
- }
-
- public static native void setupTest();
- public static native void setupThread(Thread t, List<String> events, Method target);
-}
diff --git a/test/Android.bp b/test/Android.bp
index 380a25d..8b40d3c 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -293,7 +293,6 @@
"1951-monitor-enter-no-suspend/raw_monitor.cc",
"1953-pop-frame/pop_frame.cc",
"1957-error-ext/lasterror.cc",
- "1962-multi-thread-events/multi_thread_events.cc",
],
// Use NDK-compatible headers for ctstiagent.
header_libs: [
@@ -683,7 +682,6 @@
"1943-suspend-raw-monitor-wait/src/art/Test1943.java",
"1953-pop-frame/src/art/Test1953.java",
"1958-transform-try-jit/src/art/Test1958.java",
- "1962-multi-thread-events/src/art/Test1962.java",
],
}