Merge "Revert "Make JVMTI DisposeEnvironment and GetEnv thread safe.""
diff --git a/openjdkjvmti/art_jvmti.h b/openjdkjvmti/art_jvmti.h
index e8e62c2..682b82b 100644
--- a/openjdkjvmti/art_jvmti.h
+++ b/openjdkjvmti/art_jvmti.h
@@ -94,10 +94,6 @@
static ArtJvmTiEnv* AsArtJvmTiEnv(jvmtiEnv* env) {
return art::down_cast<ArtJvmTiEnv*>(env);
}
-
- // Top level lock. Nothing can be held when we get this except for mutator lock for full
- // thread-suspension.
- static art::Mutex *gEnvMutex ACQUIRED_AFTER(art::Locks::mutator_lock_);
};
// Macro and constexpr to make error values less annoying to write.
diff --git a/openjdkjvmti/events-inl.h b/openjdkjvmti/events-inl.h
index 007669b..5344e0f 100644
--- a/openjdkjvmti/events-inl.h
+++ b/openjdkjvmti/events-inl.h
@@ -46,45 +46,6 @@
namespace impl {
-// Helper for ensuring that the dispatch environment is sane. Events with JNIEnvs need to stash
-// pending exceptions since they can cause new ones to be thrown. In accordance with the JVMTI
-// specification we allow exceptions originating from events to overwrite the current exception,
-// including exceptions originating from earlier events.
-class ScopedEventDispatchEnvironment FINAL : public art::ValueObject {
- public:
- ScopedEventDispatchEnvironment() : env_(nullptr), throw_(nullptr, nullptr) {
- DCHECK_EQ(art::Thread::Current()->GetState(), art::ThreadState::kNative);
- }
-
- explicit ScopedEventDispatchEnvironment(JNIEnv* env)
- : env_(env),
- throw_(env_, env_->ExceptionOccurred()) {
- DCHECK_EQ(art::Thread::Current()->GetState(), art::ThreadState::kNative);
- // The spec doesn't say how much local data should be there, so we just give 128 which seems
- // likely to be enough for most cases.
- env_->PushLocalFrame(128);
- env_->ExceptionClear();
- }
-
- ~ScopedEventDispatchEnvironment() {
- if (env_ != nullptr) {
- if (throw_.get() != nullptr && !env_->ExceptionCheck()) {
- // TODO It would be nice to add the overwritten exceptions to the suppressed exceptions list
- // of the newest exception.
- env_->Throw(throw_.get());
- }
- env_->PopLocalFrame(nullptr);
- }
- DCHECK_EQ(art::Thread::Current()->GetState(), art::ThreadState::kNative);
- }
-
- private:
- JNIEnv* env_;
- ScopedLocalRef<jthrowable> throw_;
-
- DISALLOW_COPY_AND_ASSIGN(ScopedEventDispatchEnvironment);
-};
-
// Infrastructure to achieve type safety for event dispatch.
#define FORALL_EVENT_TYPES(fn) \
@@ -136,68 +97,27 @@
#undef EVENT_FN_TYPE
-#define MAKE_EVENT_HANDLER_FUNC(name, enum_name) \
-template<> \
-struct EventHandlerFunc<enum_name> { \
- using EventFnType = typename impl::EventFnType<enum_name>::type; \
- explicit EventHandlerFunc(ArtJvmTiEnv* env) \
- : env_(env), \
- fn_(env_->event_callbacks == nullptr ? nullptr : env_->event_callbacks->name) { } \
- \
- template <typename ...Args> \
- ALWAYS_INLINE \
- void ExecuteCallback(JNIEnv* jnienv, Args... args) const { \
- if (fn_ != nullptr) { \
- ScopedEventDispatchEnvironment sede(jnienv); \
- DoExecute(jnienv, args...); \
- } \
- } \
- \
- template <typename ...Args> \
- ALWAYS_INLINE \
- void ExecuteCallback(Args... args) const { \
- if (fn_ != nullptr) { \
- ScopedEventDispatchEnvironment sede; \
- DoExecute(args...); \
- } \
- } \
- \
- private: \
- template <typename ...Args> \
- ALWAYS_INLINE \
- inline void DoExecute(Args... args) const { \
- static_assert(std::is_same<EventFnType, void(*)(jvmtiEnv*, Args...)>::value, \
- "Unexpected different type of ExecuteCallback"); \
- fn_(env_, args...); \
- } \
- \
- public: \
- ArtJvmTiEnv* env_; \
- EventFnType fn_; \
-};
+template <ArtJvmtiEvent kEvent>
+ALWAYS_INLINE inline typename EventFnType<kEvent>::type GetCallback(ArtJvmTiEnv* env);
-FORALL_EVENT_TYPES(MAKE_EVENT_HANDLER_FUNC)
+#define GET_CALLBACK(name, enum_name) \
+template <> \
+ALWAYS_INLINE inline EventFnType<enum_name>::type GetCallback<enum_name>( \
+ ArtJvmTiEnv* env) { \
+ if (env->event_callbacks == nullptr) { \
+ return nullptr; \
+ } \
+ return env->event_callbacks->name; \
+}
-#undef MAKE_EVENT_HANDLER_FUNC
+FORALL_EVENT_TYPES(GET_CALLBACK)
+
+#undef GET_CALLBACK
#undef FORALL_EVENT_TYPES
} // namespace impl
-template <ArtJvmtiEvent kEvent, typename ...Args>
-inline std::vector<impl::EventHandlerFunc<kEvent>> EventHandler::CollectEvents(art::Thread* thread,
- Args... args) const {
- art::MutexLock mu(thread, envs_lock_);
- std::vector<impl::EventHandlerFunc<kEvent>> handlers;
- for (ArtJvmTiEnv* env : envs) {
- if (ShouldDispatch<kEvent>(env, thread, args...)) {
- impl::EventHandlerFunc<kEvent> h(env);
- handlers.push_back(h);
- }
- }
- return handlers;
-}
-
// C++ does not allow partial template function specialization. The dispatch for our separated
// ClassFileLoadHook event types is the same, so use this helper for code deduplication.
template <ArtJvmtiEvent kEvent>
@@ -211,37 +131,29 @@
const unsigned char* class_data,
jint* new_class_data_len,
unsigned char** new_class_data) const {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
static_assert(kEvent == ArtJvmtiEvent::kClassFileLoadHookRetransformable ||
kEvent == ArtJvmtiEvent::kClassFileLoadHookNonRetransformable, "Unsupported event");
DCHECK(*new_class_data == nullptr);
jint current_len = class_data_len;
unsigned char* current_class_data = const_cast<unsigned char*>(class_data);
- std::vector<impl::EventHandlerFunc<kEvent>> handlers =
- CollectEvents<kEvent>(thread,
- jnienv,
- class_being_redefined,
- loader,
- name,
- protection_domain,
- class_data_len,
- class_data,
- new_class_data_len,
- new_class_data);
ArtJvmTiEnv* last_env = nullptr;
- for (const impl::EventHandlerFunc<kEvent>& event : handlers) {
+ for (ArtJvmTiEnv* env : envs) {
+ if (env == nullptr) {
+ continue;
+ }
jint new_len = 0;
unsigned char* new_data = nullptr;
- ExecuteCallback<kEvent>(event,
- jnienv,
- class_being_redefined,
- loader,
- name,
- protection_domain,
- current_len,
- static_cast<const unsigned char*>(current_class_data),
- &new_len,
- &new_data);
+ DispatchEventOnEnv<kEvent>(env,
+ thread,
+ jnienv,
+ class_being_redefined,
+ loader,
+ name,
+ protection_domain,
+ current_len,
+ static_cast<const unsigned char*>(current_class_data),
+ &new_len,
+ &new_data);
if (new_data != nullptr && new_data != current_class_data) {
// Destroy the data the last transformer made. We skip this if the previous state was the
// initial one since we don't know here which jvmtiEnv allocated it.
@@ -250,7 +162,7 @@
if (last_env != nullptr) {
last_env->Deallocate(current_class_data);
}
- last_env = event.env_;
+ last_env = env;
current_class_data = new_data;
current_len = new_len;
}
@@ -265,27 +177,69 @@
// exactly the argument types of the corresponding Jvmti kEvent function pointer.
template <ArtJvmtiEvent kEvent, typename ...Args>
+inline void EventHandler::ExecuteCallback(ArtJvmTiEnv* env, Args... args) {
+ using FnType = typename impl::EventFnType<kEvent>::type;
+ FnType callback = impl::GetCallback<kEvent>(env);
+ if (callback != nullptr) {
+ (*callback)(env, args...);
+ }
+}
+
+template <ArtJvmtiEvent kEvent, typename ...Args>
inline void EventHandler::DispatchEvent(art::Thread* thread, Args... args) const {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
static_assert(!std::is_same<JNIEnv*,
typename std::decay_t<
std::tuple_element_t<0, std::tuple<Args..., nullptr_t>>>>::value,
"Should be calling DispatchEvent with explicit JNIEnv* argument!");
DCHECK(thread == nullptr || !thread->IsExceptionPending());
- std::vector<impl::EventHandlerFunc<kEvent>> events = CollectEvents<kEvent>(thread, args...);
- for (auto event : events) {
- ExecuteCallback<kEvent>(event, args...);
+ for (ArtJvmTiEnv* env : envs) {
+ if (env != nullptr) {
+ DispatchEventOnEnv<kEvent, Args...>(env, thread, args...);
+ }
}
}
+// Helper for ensuring that the dispatch environment is sane. Events with JNIEnvs need to stash
+// pending exceptions since they can cause new ones to be thrown. In accordance with the JVMTI
+// specification we allow exceptions originating from events to overwrite the current exception,
+// including exceptions originating from earlier events.
+class ScopedEventDispatchEnvironment FINAL : public art::ValueObject {
+ public:
+ explicit ScopedEventDispatchEnvironment(JNIEnv* env)
+ : env_(env),
+ thr_(env_, env_->ExceptionOccurred()),
+ suspend_(art::Thread::Current(), art::kNative) {
+ // The spec doesn't say how much local data should be there, so we just give 128 which seems
+ // likely to be enough for most cases.
+ env_->PushLocalFrame(128);
+ env_->ExceptionClear();
+ UNUSED(suspend_);
+ }
+
+ ~ScopedEventDispatchEnvironment() {
+ if (thr_.get() != nullptr && !env_->ExceptionCheck()) {
+ // TODO It would be nice to add the overwritten exceptions to the suppressed exceptions list
+ // of the newest exception.
+ env_->Throw(thr_.get());
+ }
+ env_->PopLocalFrame(nullptr);
+ }
+
+ private:
+ JNIEnv* env_;
+ ScopedLocalRef<jthrowable> thr_;
+ // Not actually unused. The destructor/constructor does important work.
+ art::ScopedThreadStateChange suspend_;
+
+ DISALLOW_COPY_AND_ASSIGN(ScopedEventDispatchEnvironment);
+};
+
template <ArtJvmtiEvent kEvent, typename ...Args>
inline void EventHandler::DispatchEvent(art::Thread* thread, JNIEnv* jnienv, Args... args) const {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
- std::vector<impl::EventHandlerFunc<kEvent>> events = CollectEvents<kEvent>(thread,
- jnienv,
- args...);
- for (auto event : events) {
- ExecuteCallback<kEvent>(event, jnienv, args...);
+ for (ArtJvmTiEnv* env : envs) {
+ if (env != nullptr) {
+ DispatchEventOnEnv<kEvent, Args...>(env, thread, jnienv, args...);
+ }
}
}
@@ -294,9 +248,8 @@
ArtJvmTiEnv* env, art::Thread* thread, JNIEnv* jnienv, Args... args) const {
DCHECK(env != nullptr);
if (ShouldDispatch<kEvent, JNIEnv*, Args...>(env, thread, jnienv, args...)) {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
- impl::EventHandlerFunc<kEvent> func(env);
- ExecuteCallback<kEvent>(func, jnienv, args...);
+ ScopedEventDispatchEnvironment sede(jnienv);
+ ExecuteCallback<kEvent, JNIEnv*, Args...>(env, jnienv, args...);
}
}
@@ -307,26 +260,11 @@
typename std::decay_t<
std::tuple_element_t<0, std::tuple<Args..., nullptr_t>>>>::value,
"Should be calling DispatchEventOnEnv with explicit JNIEnv* argument!");
- DCHECK(env != nullptr);
- if (ShouldDispatch<kEvent, Args...>(env, thread, args...)) {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
- impl::EventHandlerFunc<kEvent> func(env);
- ExecuteCallback<kEvent>(func, args...);
+ if (ShouldDispatch<kEvent>(env, thread, args...)) {
+ ExecuteCallback<kEvent, Args...>(env, args...);
}
}
-template <ArtJvmtiEvent kEvent, typename ...Args>
-inline void EventHandler::ExecuteCallback(impl::EventHandlerFunc<kEvent> handler, Args... args) {
- handler.ExecuteCallback(args...);
-}
-
-template <ArtJvmtiEvent kEvent, typename ...Args>
-inline void EventHandler::ExecuteCallback(impl::EventHandlerFunc<kEvent> handler,
- JNIEnv* jnienv,
- Args... args) {
- handler.ExecuteCallback(jnienv, args...);
-}
-
// Events that need custom logic for if we send the event but are otherwise normal. This includes
// the kBreakpoint, kFramePop, kFieldAccess, and kFieldModification events.
@@ -409,13 +347,14 @@
// something.
template <>
inline void EventHandler::ExecuteCallback<ArtJvmtiEvent::kFramePop>(
- impl::EventHandlerFunc<ArtJvmtiEvent::kFramePop> event,
+ ArtJvmTiEnv* env,
JNIEnv* jnienv,
jthread jni_thread,
jmethodID jmethod,
jboolean is_exception,
const art::ShadowFrame* frame ATTRIBUTE_UNUSED) {
- ExecuteCallback<ArtJvmtiEvent::kFramePop>(event, jnienv, jni_thread, jmethod, is_exception);
+ ExecuteCallback<ArtJvmtiEvent::kFramePop>(
+ env, jnienv, jni_thread, jmethod, is_exception);
}
// Need to give a custom specialization for NativeMethodBind since it has to deal with an out
@@ -427,25 +366,20 @@
jmethodID method,
void* cur_method,
void** new_method) const {
- art::ScopedThreadStateChange stsc(thread, art::ThreadState::kNative);
- std::vector<impl::EventHandlerFunc<ArtJvmtiEvent::kNativeMethodBind>> events =
- CollectEvents<ArtJvmtiEvent::kNativeMethodBind>(thread,
- jnienv,
- jni_thread,
- method,
- cur_method,
- new_method);
*new_method = cur_method;
- for (auto event : events) {
- *new_method = cur_method;
- ExecuteCallback<ArtJvmtiEvent::kNativeMethodBind>(event,
- jnienv,
- jni_thread,
- method,
- cur_method,
- new_method);
- if (*new_method != nullptr) {
- cur_method = *new_method;
+ for (ArtJvmTiEnv* env : envs) {
+ if (env != nullptr) {
+ *new_method = cur_method;
+ DispatchEventOnEnv<ArtJvmtiEvent::kNativeMethodBind>(env,
+ thread,
+ jnienv,
+ jni_thread,
+ method,
+ cur_method,
+ new_method);
+ if (*new_method != nullptr) {
+ cur_method = *new_method;
+ }
}
}
*new_method = cur_method;
@@ -505,7 +439,7 @@
}
template <ArtJvmtiEvent kEvent>
-inline bool EventHandler::ShouldDispatchOnThread(ArtJvmTiEnv* env, art::Thread* thread) const {
+inline bool EventHandler::ShouldDispatchOnThread(ArtJvmTiEnv* env, art::Thread* thread) {
bool dispatch = env->event_masks.global_event_mask.Test(kEvent);
if (!dispatch && thread != nullptr && env->event_masks.unioned_thread_event_mask.Test(kEvent)) {
@@ -527,11 +461,6 @@
}
inline void EventHandler::RecalculateGlobalEventMask(ArtJvmtiEvent event) {
- art::MutexLock mu(art::Thread::Current(), envs_lock_);
- RecalculateGlobalEventMaskLocked(event);
-}
-
-inline void EventHandler::RecalculateGlobalEventMaskLocked(ArtJvmtiEvent event) {
bool union_value = false;
for (const ArtJvmTiEnv* stored_env : envs) {
if (stored_env == nullptr) {
diff --git a/openjdkjvmti/events.cc b/openjdkjvmti/events.cc
index be4ebbc..d1d606d 100644
--- a/openjdkjvmti/events.cc
+++ b/openjdkjvmti/events.cc
@@ -193,21 +193,25 @@
}
void EventHandler::RegisterArtJvmTiEnv(ArtJvmTiEnv* env) {
- art::MutexLock mu(art::Thread::Current(), envs_lock_);
- envs.push_back(env);
+ // Since we never shrink this array we might as well try to fill gaps.
+ auto it = std::find(envs.begin(), envs.end(), nullptr);
+ if (it != envs.end()) {
+ *it = env;
+ } else {
+ envs.push_back(env);
+ }
}
void EventHandler::RemoveArtJvmTiEnv(ArtJvmTiEnv* env) {
- art::MutexLock mu(art::Thread::Current(), envs_lock_);
// Since we might be currently iterating over the envs list we cannot actually erase elements.
// Instead we will simply replace them with 'nullptr' and skip them manually.
auto it = std::find(envs.begin(), envs.end(), env);
if (it != envs.end()) {
- envs.erase(it);
+ *it = nullptr;
for (size_t i = static_cast<size_t>(ArtJvmtiEvent::kMinEventTypeVal);
i <= static_cast<size_t>(ArtJvmtiEvent::kMaxEventTypeVal);
++i) {
- RecalculateGlobalEventMaskLocked(static_cast<ArtJvmtiEvent>(i));
+ RecalculateGlobalEventMask(static_cast<ArtJvmtiEvent>(i));
}
}
}
@@ -427,11 +431,11 @@
finish_enabled_(false) {}
void StartPause() OVERRIDE {
- handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionStart>(art::Thread::Current());
+ handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionStart>(nullptr);
}
void EndPause() OVERRIDE {
- handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionFinish>(art::Thread::Current());
+ handler_->DispatchEvent<ArtJvmtiEvent::kGarbageCollectionFinish>(nullptr);
}
bool IsEnabled() {
@@ -1172,8 +1176,7 @@
art::Runtime::Current()->GetInstrumentation()->RemoveListener(method_trace_listener_.get(), ~0);
}
-EventHandler::EventHandler() : envs_lock_("JVMTI Environment List Lock",
- art::LockLevel::kTopLockLevel) {
+EventHandler::EventHandler() {
alloc_listener_.reset(new JvmtiAllocationListener(this));
ddm_listener_.reset(new JvmtiDdmChunkListener(this));
gc_pause_listener_.reset(new JvmtiGcPauseListener(this));
diff --git a/openjdkjvmti/events.h b/openjdkjvmti/events.h
index c73215f..a99ed7b 100644
--- a/openjdkjvmti/events.h
+++ b/openjdkjvmti/events.h
@@ -158,10 +158,6 @@
void HandleChangedCapabilities(const jvmtiCapabilities& caps, bool caps_added);
};
-namespace impl {
-template <ArtJvmtiEvent kEvent> struct EventHandlerFunc { };
-} // namespace impl
-
// Helper class for event handling.
class EventHandler {
public:
@@ -173,10 +169,10 @@
// Register an env. It is assumed that this happens on env creation, that is, no events are
// enabled, yet.
- void RegisterArtJvmTiEnv(ArtJvmTiEnv* env) REQUIRES(!envs_lock_);
+ void RegisterArtJvmTiEnv(ArtJvmTiEnv* env);
// Remove an env.
- void RemoveArtJvmTiEnv(ArtJvmTiEnv* env) REQUIRES(!envs_lock_);
+ void RemoveArtJvmTiEnv(ArtJvmTiEnv* env);
bool IsEventEnabledAnywhere(ArtJvmtiEvent event) const {
if (!EventMask::EventIsInRange(event)) {
@@ -188,15 +184,13 @@
jvmtiError SetEvent(ArtJvmTiEnv* env,
art::Thread* thread,
ArtJvmtiEvent event,
- jvmtiEventMode mode)
- REQUIRES(!envs_lock_);
+ jvmtiEventMode mode);
// Dispatch event to all registered environments. Since this one doesn't have a JNIEnv* it doesn't
// matter if it has the mutator_lock.
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- inline void DispatchEvent(art::Thread* thread, Args... args) const
- REQUIRES(!envs_lock_);
+ inline void DispatchEvent(art::Thread* thread, Args... args) const;
// Dispatch event to all registered environments stashing exceptions as needed. This works since
// JNIEnv* is always the second argument if it is passed to an event. Needed since C++ does not
@@ -206,8 +200,7 @@
// the event to allocate local references.
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- inline void DispatchEvent(art::Thread* thread, JNIEnv* jnienv, Args... args) const
- REQUIRES(!envs_lock_);
+ inline void DispatchEvent(art::Thread* thread, JNIEnv* jnienv, Args... args) const;
// Tell the event handler capabilities were added/lost so it can adjust the sent events.If
// caps_added is true then caps is all the newly set capabilities of the jvmtiEnv. If it is false
@@ -215,50 +208,30 @@
ALWAYS_INLINE
inline void HandleChangedCapabilities(ArtJvmTiEnv* env,
const jvmtiCapabilities& caps,
- bool added)
- REQUIRES(!envs_lock_);
+ bool added);
// Dispatch event to the given environment, only.
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- inline void DispatchEventOnEnv(ArtJvmTiEnv* env,
- art::Thread* thread,
- JNIEnv* jnienv,
- Args... args) const
- REQUIRES(!envs_lock_);
+ inline void DispatchEventOnEnv(
+ ArtJvmTiEnv* env, art::Thread* thread, JNIEnv* jnienv, Args... args) const;
// Dispatch event to the given environment, only.
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- inline void DispatchEventOnEnv(ArtJvmTiEnv* env, art::Thread* thread, Args... args) const
- REQUIRES(!envs_lock_);
+ inline void DispatchEventOnEnv(ArtJvmTiEnv* env, art::Thread* thread, Args... args) const;
private:
- template <ArtJvmtiEvent kEvent, typename ...Args>
- ALWAYS_INLINE
- inline std::vector<impl::EventHandlerFunc<kEvent>> CollectEvents(art::Thread* thread,
- Args... args) const
- REQUIRES(!envs_lock_);
-
template <ArtJvmtiEvent kEvent>
ALWAYS_INLINE
- inline bool ShouldDispatchOnThread(ArtJvmTiEnv* env, art::Thread* thread) const;
+ static inline bool ShouldDispatchOnThread(ArtJvmTiEnv* env, art::Thread* thread);
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- static inline void ExecuteCallback(impl::EventHandlerFunc<kEvent> handler,
- JNIEnv* env,
- Args... args)
- REQUIRES(!envs_lock_);
+ static inline void ExecuteCallback(ArtJvmTiEnv* env, Args... args);
template <ArtJvmtiEvent kEvent, typename ...Args>
ALWAYS_INLINE
- static inline void ExecuteCallback(impl::EventHandlerFunc<kEvent> handler, Args... args)
- REQUIRES(!envs_lock_);
-
- // Public for use to collect dispatches
- template <ArtJvmtiEvent kEvent, typename ...Args>
- ALWAYS_INLINE
inline bool ShouldDispatch(ArtJvmTiEnv* env, art::Thread* thread, Args... args) const;
ALWAYS_INLINE
@@ -268,9 +241,7 @@
// Recalculates the event mask for the given event.
ALWAYS_INLINE
- inline void RecalculateGlobalEventMask(ArtJvmtiEvent event) REQUIRES(!envs_lock_);
- ALWAYS_INLINE
- inline void RecalculateGlobalEventMaskLocked(ArtJvmtiEvent event) REQUIRES(envs_lock_);
+ inline void RecalculateGlobalEventMask(ArtJvmtiEvent event);
template <ArtJvmtiEvent kEvent>
ALWAYS_INLINE inline void DispatchClassFileLoadHookEvent(art::Thread* thread,
@@ -282,8 +253,7 @@
jint class_data_len,
const unsigned char* class_data,
jint* new_class_data_len,
- unsigned char** new_class_data) const
- REQUIRES(!envs_lock_);
+ unsigned char** new_class_data) const;
void HandleEventType(ArtJvmtiEvent event, bool enable);
void HandleLocalAccessCapabilityAdded();
@@ -291,13 +261,10 @@
bool OtherMonitorEventsEnabledAnywhere(ArtJvmtiEvent event);
- // List of all JvmTiEnv objects that have been created, in their creation order. It is a std::list
- // since we mostly access it by iterating over the entire thing, only ever append to the end, and
- // need to be able to remove arbitrary elements from it.
- std::list<ArtJvmTiEnv*> envs GUARDED_BY(envs_lock_);
-
- // Top level lock. Nothing at all should be held when we lock this.
- mutable art::Mutex envs_lock_ ACQUIRED_BEFORE(art::Locks::instrument_entrypoints_lock_);
+ // List of all JvmTiEnv objects that have been created, in their creation order.
+ // NB Some elements might be null representing envs that have been deleted. They should be skipped
+ // anytime this list is used.
+ std::vector<ArtJvmTiEnv*> envs;
// A union of all enabled events, anywhere.
EventMask global_mask;
diff --git a/openjdkjvmti/object_tagging.cc b/openjdkjvmti/object_tagging.cc
index ba242ef..6ba7165 100644
--- a/openjdkjvmti/object_tagging.cc
+++ b/openjdkjvmti/object_tagging.cc
@@ -61,8 +61,7 @@
return event_handler_->IsEventEnabledAnywhere(ArtJvmtiEvent::kObjectFree);
}
void ObjectTagTable::HandleNullSweep(jlong tag) {
- event_handler_->DispatchEventOnEnv<ArtJvmtiEvent::kObjectFree>(
- jvmti_env_, art::Thread::Current(), tag);
+ event_handler_->DispatchEventOnEnv<ArtJvmtiEvent::kObjectFree>(jvmti_env_, nullptr, tag);
}
} // namespace openjdkjvmti
diff --git a/openjdkjvmti/ti_dump.cc b/openjdkjvmti/ti_dump.cc
index 253580e..809a5e4 100644
--- a/openjdkjvmti/ti_dump.cc
+++ b/openjdkjvmti/ti_dump.cc
@@ -47,7 +47,7 @@
void SigQuit() OVERRIDE REQUIRES_SHARED(art::Locks::mutator_lock_) {
art::Thread* thread = art::Thread::Current();
art::ScopedThreadSuspension sts(thread, art::ThreadState::kNative);
- event_handler->DispatchEvent<ArtJvmtiEvent::kDataDumpRequest>(art::Thread::Current());
+ event_handler->DispatchEvent<ArtJvmtiEvent::kDataDumpRequest>(nullptr);
}
EventHandler* event_handler = nullptr;
diff --git a/openjdkjvmti/ti_phase.cc b/openjdkjvmti/ti_phase.cc
index 7157974..23df27f 100644
--- a/openjdkjvmti/ti_phase.cc
+++ b/openjdkjvmti/ti_phase.cc
@@ -57,7 +57,6 @@
}
void NextRuntimePhase(RuntimePhase phase) REQUIRES_SHARED(art::Locks::mutator_lock_) OVERRIDE {
- art::Thread* self = art::Thread::Current();
switch (phase) {
case RuntimePhase::kInitialAgents:
PhaseUtil::current_phase_ = JVMTI_PHASE_PRIMORDIAL;
@@ -65,7 +64,8 @@
case RuntimePhase::kStart:
{
PhaseUtil::current_phase_ = JVMTI_PHASE_START;
- event_handler->DispatchEvent<ArtJvmtiEvent::kVmStart>(self, GetJniEnv());
+ art::ScopedThreadSuspension sts(art::Thread::Current(), art::ThreadState::kNative);
+ event_handler->DispatchEvent<ArtJvmtiEvent::kVmStart>(nullptr, GetJniEnv());
}
break;
case RuntimePhase::kInit:
@@ -74,7 +74,9 @@
PhaseUtil::current_phase_ = JVMTI_PHASE_LIVE;
{
ScopedLocalRef<jthread> thread(GetJniEnv(), GetCurrentJThread());
- event_handler->DispatchEvent<ArtJvmtiEvent::kVmInit>(self, GetJniEnv(), thread.get());
+ art::ScopedThreadSuspension sts(art::Thread::Current(), art::ThreadState::kNative);
+ event_handler->DispatchEvent<ArtJvmtiEvent::kVmInit>(
+ nullptr, GetJniEnv(), thread.get());
}
// We need to have these events be ordered to match behavior expected by some real-world
// agents. The spec does not really require this but compatibility is a useful property to
@@ -84,7 +86,8 @@
break;
case RuntimePhase::kDeath:
{
- event_handler->DispatchEvent<ArtJvmtiEvent::kVmDeath>(self, GetJniEnv());
+ art::ScopedThreadSuspension sts(art::Thread::Current(), art::ThreadState::kNative);
+ event_handler->DispatchEvent<ArtJvmtiEvent::kVmDeath>(nullptr, GetJniEnv());
PhaseUtil::current_phase_ = JVMTI_PHASE_DEAD;
}
// TODO: Block events now.
diff --git a/runtime/base/mutex-inl.h b/runtime/base/mutex-inl.h
index 39ad3d0..c9d48ff 100644
--- a/runtime/base/mutex-inl.h
+++ b/runtime/base/mutex-inl.h
@@ -80,9 +80,7 @@
// (see Thread::TransitionFromSuspendedToRunnable).
level == kThreadSuspendCountLock ||
// Avoid recursive death.
- level == kAbortLock ||
- // Locks at the absolute top of the stack can be locked at any time.
- level == kTopLockLevel) << level;
+ level == kAbortLock) << level;
}
}
diff --git a/runtime/base/mutex.h b/runtime/base/mutex.h
index 43ea3a2..87c4afe 100644
--- a/runtime/base/mutex.h
+++ b/runtime/base/mutex.h
@@ -122,11 +122,6 @@
kInstrumentEntrypointsLock,
kZygoteCreationLock,
- // The highest valid lock level. Use this if there is code that should only be called with no
- // other locks held. Since this is the highest lock level we also allow it to be held even if the
- // runtime or current thread is not fully set-up yet (for example during thread attach).
- kTopLockLevel,
-
kLockLevelCount // Must come last.
};
std::ostream& operator<<(std::ostream& os, const LockLevel& rhs);
diff --git a/runtime/ti/agent.cc b/runtime/ti/agent.cc
index 548752e..3bf169a 100644
--- a/runtime/ti/agent.cc
+++ b/runtime/ti/agent.cc
@@ -21,8 +21,6 @@
#include "base/strlcpy.h"
#include "java_vm_ext.h"
#include "runtime.h"
-#include "thread-current-inl.h"
-#include "scoped_thread_state_change-inl.h"
namespace art {
namespace ti {
@@ -37,7 +35,6 @@
Agent::LoadError Agent::DoLoadHelper(bool attaching,
/*out*/jint* call_res,
/*out*/std::string* error_msg) {
- ScopedThreadStateChange stsc(Thread::Current(), ThreadState::kNative);
DCHECK(call_res != nullptr);
DCHECK(error_msg != nullptr);
diff --git a/test/1941-dispose-stress/dispose_stress.cc b/test/1941-dispose-stress/dispose_stress.cc
deleted file mode 100644
index e8fcc77..0000000
--- a/test/1941-dispose-stress/dispose_stress.cc
+++ /dev/null
@@ -1,59 +0,0 @@
-/*
- * 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.
- */
-
-#include <atomic>
-
-#include "android-base/logging.h"
-#include "jni.h"
-#include "scoped_local_ref.h"
-#include "scoped_primitive_array.h"
-
-#include "jvmti.h"
-
-// Test infrastructure
-#include "jvmti_helper.h"
-#include "test_env.h"
-
-namespace art {
-namespace Test1941DisposeStress {
-
-extern "C" JNIEXPORT jlong JNICALL Java_art_Test1941_AllocEnv(JNIEnv* env, jclass) {
- JavaVM* vm = nullptr;
- if (env->GetJavaVM(&vm) != 0) {
- ScopedLocalRef<jclass> rt_exception(env, env->FindClass("java/lang/RuntimeException"));
- env->ThrowNew(rt_exception.get(), "Unable to get JavaVM");
- return -1;
- }
- jvmtiEnv* new_env = nullptr;
- if (vm->GetEnv(reinterpret_cast<void**>(&new_env), JVMTI_VERSION_1_0) != 0) {
- ScopedLocalRef<jclass> rt_exception(env, env->FindClass("java/lang/RuntimeException"));
- env->ThrowNew(rt_exception.get(), "Unable to create new jvmtiEnv");
- return -1;
- }
- return static_cast<jlong>(reinterpret_cast<intptr_t>(new_env));
-}
-
-extern "C" JNIEXPORT void JNICALL Java_art_Test1941_FreeEnv(JNIEnv* env,
- jclass,
- jlong jvmti_env_ptr) {
- JvmtiErrorToException(env,
- jvmti_env,
- reinterpret_cast<jvmtiEnv*>(jvmti_env_ptr)->DisposeEnvironment());
-}
-
-} // namespace Test1941DisposeStress
-} // namespace art
-
diff --git a/test/1941-dispose-stress/expected.txt b/test/1941-dispose-stress/expected.txt
deleted file mode 100644
index ca2eddc..0000000
--- a/test/1941-dispose-stress/expected.txt
+++ /dev/null
@@ -1 +0,0 @@
-fib(20) is 6765
diff --git a/test/1941-dispose-stress/info.txt b/test/1941-dispose-stress/info.txt
deleted file mode 100644
index e4a584e..0000000
--- a/test/1941-dispose-stress/info.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-Test basic JVMTI single step functionality.
-
-Ensures that we can receive single step events from JVMTI.
diff --git a/test/1941-dispose-stress/run b/test/1941-dispose-stress/run
deleted file mode 100755
index 51875a7..0000000
--- a/test/1941-dispose-stress/run
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/bash
-#
-# Copyright 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.
-
-# Ask for stack traces to be dumped to a file rather than to stdout.
-./default-run "$@" --jvmti
diff --git a/test/1941-dispose-stress/src/Main.java b/test/1941-dispose-stress/src/Main.java
deleted file mode 100644
index 2fe6b81..0000000
--- a/test/1941-dispose-stress/src/Main.java
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * 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.Test1941.run();
- }
-}
diff --git a/test/1941-dispose-stress/src/art/Breakpoint.java b/test/1941-dispose-stress/src/art/Breakpoint.java
deleted file mode 100644
index bbb89f7..0000000
--- a/test/1941-dispose-stress/src/art/Breakpoint.java
+++ /dev/null
@@ -1,202 +0,0 @@
-/*
- * 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.
- */
-
-package art;
-
-import java.lang.reflect.Executable;
-import java.util.HashSet;
-import java.util.Set;
-import java.util.Objects;
-
-public class Breakpoint {
- public static class Manager {
- public static class BP {
- public final Executable method;
- public final long location;
-
- public BP(Executable method) {
- this(method, getStartLocation(method));
- }
-
- public BP(Executable method, long location) {
- this.method = method;
- this.location = location;
- }
-
- @Override
- public boolean equals(Object other) {
- return (other instanceof BP) &&
- method.equals(((BP)other).method) &&
- location == ((BP)other).location;
- }
-
- @Override
- public String toString() {
- return method.toString() + " @ " + getLine();
- }
-
- @Override
- public int hashCode() {
- return Objects.hash(method, location);
- }
-
- public int getLine() {
- try {
- LineNumber[] lines = getLineNumberTable(method);
- int best = -1;
- for (LineNumber l : lines) {
- if (l.location > location) {
- break;
- } else {
- best = l.line;
- }
- }
- return best;
- } catch (Exception e) {
- return -1;
- }
- }
- }
-
- private Set<BP> breaks = new HashSet<>();
-
- public void setBreakpoints(BP... bs) {
- for (BP b : bs) {
- if (breaks.add(b)) {
- Breakpoint.setBreakpoint(b.method, b.location);
- }
- }
- }
- public void setBreakpoint(Executable method, long location) {
- setBreakpoints(new BP(method, location));
- }
-
- public void clearBreakpoints(BP... bs) {
- for (BP b : bs) {
- if (breaks.remove(b)) {
- Breakpoint.clearBreakpoint(b.method, b.location);
- }
- }
- }
- public void clearBreakpoint(Executable method, long location) {
- clearBreakpoints(new BP(method, location));
- }
-
- public void clearAllBreakpoints() {
- clearBreakpoints(breaks.toArray(new BP[0]));
- }
- }
-
- public static void startBreakpointWatch(Class<?> methodClass,
- Executable breakpointReached,
- Thread thr) {
- startBreakpointWatch(methodClass, breakpointReached, false, thr);
- }
-
- /**
- * Enables the trapping of breakpoint events.
- *
- * If allowRecursive == true then breakpoints will be sent even if one is currently being handled.
- */
- public static native void startBreakpointWatch(Class<?> methodClass,
- Executable breakpointReached,
- boolean allowRecursive,
- Thread thr);
- public static native void stopBreakpointWatch(Thread thr);
-
- public static final class LineNumber implements Comparable<LineNumber> {
- public final long location;
- public final int line;
-
- private LineNumber(long loc, int line) {
- this.location = loc;
- this.line = line;
- }
-
- public boolean equals(Object other) {
- return other instanceof LineNumber && ((LineNumber)other).line == line &&
- ((LineNumber)other).location == location;
- }
-
- public int compareTo(LineNumber other) {
- int v = Integer.valueOf(line).compareTo(Integer.valueOf(other.line));
- if (v != 0) {
- return v;
- } else {
- return Long.valueOf(location).compareTo(Long.valueOf(other.location));
- }
- }
- }
-
- public static native void setBreakpoint(Executable m, long loc);
- public static void setBreakpoint(Executable m, LineNumber l) {
- setBreakpoint(m, l.location);
- }
-
- public static native void clearBreakpoint(Executable m, long loc);
- public static void clearBreakpoint(Executable m, LineNumber l) {
- clearBreakpoint(m, l.location);
- }
-
- private static native Object[] getLineNumberTableNative(Executable m);
- public static LineNumber[] getLineNumberTable(Executable m) {
- Object[] nativeTable = getLineNumberTableNative(m);
- long[] location = (long[])(nativeTable[0]);
- int[] lines = (int[])(nativeTable[1]);
- if (lines.length != location.length) {
- throw new Error("Lines and locations have different lengths!");
- }
- LineNumber[] out = new LineNumber[lines.length];
- for (int i = 0; i < lines.length; i++) {
- out[i] = new LineNumber(location[i], lines[i]);
- }
- return out;
- }
-
- public static native long getStartLocation(Executable m);
-
- public static int locationToLine(Executable m, long location) {
- try {
- Breakpoint.LineNumber[] lines = Breakpoint.getLineNumberTable(m);
- int best = -1;
- for (Breakpoint.LineNumber l : lines) {
- if (l.location > location) {
- break;
- } else {
- best = l.line;
- }
- }
- return best;
- } catch (Exception e) {
- return -1;
- }
- }
-
- public static long lineToLocation(Executable m, int line) throws Exception {
- try {
- Breakpoint.LineNumber[] lines = Breakpoint.getLineNumberTable(m);
- for (Breakpoint.LineNumber l : lines) {
- if (l.line == line) {
- return l.location;
- }
- }
- throw new Exception("Unable to find line " + line + " in " + m);
- } catch (Exception e) {
- throw new Exception("Unable to get line number info for " + m, e);
- }
- }
-}
-
diff --git a/test/1941-dispose-stress/src/art/Test1941.java b/test/1941-dispose-stress/src/art/Test1941.java
deleted file mode 100644
index d5a9de6..0000000
--- a/test/1941-dispose-stress/src/art/Test1941.java
+++ /dev/null
@@ -1,72 +0,0 @@
-/*
- * 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.
- */
-
-package art;
-
-import java.util.Arrays;
-import java.lang.reflect.Executable;
-import java.lang.reflect.Method;
-
-public class Test1941 {
- public static final boolean PRINT_CNT = false;
- public static long CNT = 0;
-
- // Method with multiple paths we can break on.
- public static long fib(long f) {
- if (f < 0) {
- throw new IllegalArgumentException("Bad argument f < 0: f = " + f);
- } else if (f == 0) {
- return 0;
- } else if (f == 1) {
- return 1;
- } else {
- return fib(f - 1) + fib(f - 2);
- }
- }
-
- public static void notifySingleStep(Thread thr, Executable e, long loc) {
- // Don't bother actually doing anything.
- }
-
- public static void LoopAllocFreeEnv() {
- while (!Thread.interrupted()) {
- CNT++;
- long env = AllocEnv();
- FreeEnv(env);
- }
- }
-
- public static native long AllocEnv();
- public static native void FreeEnv(long env);
-
- public static void run() throws Exception {
- Thread thr = new Thread(Test1941::LoopAllocFreeEnv, "LoopNative");
- thr.start();
- Trace.enableSingleStepTracing(Test1941.class,
- Test1941.class.getDeclaredMethod(
- "notifySingleStep", Thread.class, Executable.class, Long.TYPE),
- null);
-
- System.out.println("fib(20) is " + fib(20));
-
- thr.interrupt();
- thr.join();
- Trace.disableTracing(null);
- if (PRINT_CNT) {
- System.out.println("Number of envs created/destroyed: " + CNT);
- }
- }
-}
diff --git a/test/1941-dispose-stress/src/art/Trace.java b/test/1941-dispose-stress/src/art/Trace.java
deleted file mode 100644
index 8999bb1..0000000
--- a/test/1941-dispose-stress/src/art/Trace.java
+++ /dev/null
@@ -1,68 +0,0 @@
-/*
- * 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.
- */
-
-package art;
-
-import java.lang.reflect.Field;
-import java.lang.reflect.Method;
-
-public class Trace {
- public static native void enableTracing(Class<?> methodClass,
- Method entryMethod,
- Method exitMethod,
- Method fieldAccess,
- Method fieldModify,
- Method singleStep,
- Thread thr);
- public static native void disableTracing(Thread thr);
-
- public static void enableFieldTracing(Class<?> methodClass,
- Method fieldAccess,
- Method fieldModify,
- Thread thr) {
- enableTracing(methodClass, null, null, fieldAccess, fieldModify, null, thr);
- }
-
- public static void enableMethodTracing(Class<?> methodClass,
- Method entryMethod,
- Method exitMethod,
- Thread thr) {
- enableTracing(methodClass, entryMethod, exitMethod, null, null, null, thr);
- }
-
- public static void enableSingleStepTracing(Class<?> methodClass,
- Method singleStep,
- Thread thr) {
- enableTracing(methodClass, null, null, null, null, singleStep, thr);
- }
-
- public static native void watchFieldAccess(Field f);
- public static native void watchFieldModification(Field f);
- public static native void watchAllFieldAccesses();
- public static native void watchAllFieldModifications();
-
- // the names, arguments, and even line numbers of these functions are embedded in the tests so we
- // need to add to the bottom and not modify old ones to maintain compat.
- public static native void enableTracing2(Class<?> methodClass,
- Method entryMethod,
- Method exitMethod,
- Method fieldAccess,
- Method fieldModify,
- Method singleStep,
- Method ThreadStart,
- Method ThreadEnd,
- Thread thr);
-}
diff --git a/test/Android.bp b/test/Android.bp
index 8f29251..ba24119 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -259,7 +259,6 @@
"1932-monitor-events-misc/monitor_misc.cc",
"1934-jvmti-signal-thread/signal_threads.cc",
"1939-proxy-frames/local_instance.cc",
- "1941-dispose-stress/dispose_stress.cc",
],
shared_libs: [
"libbase",