From 4d75aef609be8a12d72ed9ee4d300540f8ccd168 Mon Sep 17 00:00:00 2001 From: Richard Reingruber Date: Thu, 13 Feb 2020 15:56:28 +0100 Subject: [PATCH] 8238585: JvmtiEventControllerPrivate::enter_interp_only_mode() should not make compiled methods on stack not_entrant Reviewed-by: mdoerr, kvn, sspitsyn --- src/hotspot/share/prims/jvmtiEnvBase.hpp | 3 - .../share/prims/jvmtiEventController.cpp | 85 +++++++------------ src/hotspot/share/prims/jvmtiThreadState.cpp | 9 +- src/hotspot/share/runtime/thread.hpp | 8 +- src/hotspot/share/runtime/vmOperations.hpp | 1 - 5 files changed, 43 insertions(+), 63 deletions(-) diff --git a/src/hotspot/share/prims/jvmtiEnvBase.hpp b/src/hotspot/share/prims/jvmtiEnvBase.hpp index 7eb5a7b8b96..d60cabd8127 100644 --- a/src/hotspot/share/prims/jvmtiEnvBase.hpp +++ b/src/hotspot/share/prims/jvmtiEnvBase.hpp @@ -367,9 +367,6 @@ class VM_SetFramePop : public VM_Operation { _depth = depth; _result = JVMTI_ERROR_NONE; } - // Nested operation must be allowed for the VM_EnterInterpOnlyMode that is - // called from the JvmtiEventControllerPrivate::recompute_thread_enabled. - bool allow_nested_vm_operations() const { return true; } VMOp_Type type() const { return VMOp_SetFramePop; } jvmtiError result() { return _result; } void doit(); diff --git a/src/hotspot/share/prims/jvmtiEventController.cpp b/src/hotspot/share/prims/jvmtiEventController.cpp index 94a758f2b39..bb4b692c7ad 100644 --- a/src/hotspot/share/prims/jvmtiEventController.cpp +++ b/src/hotspot/share/prims/jvmtiEventController.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -33,7 +33,7 @@ #include "prims/jvmtiImpl.hpp" #include "prims/jvmtiThreadState.inline.hpp" #include "runtime/deoptimization.hpp" -#include "runtime/frame.hpp" +#include "runtime/frame.inline.hpp" #include "runtime/thread.inline.hpp" #include "runtime/threadSMR.hpp" #include "runtime/vframe.hpp" @@ -190,60 +190,38 @@ JvmtiEnvEventEnable::~JvmtiEnvEventEnable() { /////////////////////////////////////////////////////////////// // -// VM_EnterInterpOnlyMode +// EnterInterpOnlyModeClosure // -class VM_EnterInterpOnlyMode : public VM_Operation { -private: - JvmtiThreadState *_state; +class EnterInterpOnlyModeClosure : public HandshakeClosure { public: - VM_EnterInterpOnlyMode(JvmtiThreadState *state); - - bool allow_nested_vm_operations() const { return true; } - VMOp_Type type() const { return VMOp_EnterInterpOnlyMode; } - void doit(); - - // to do: this same function is in jvmtiImpl - should be in one place - bool can_be_deoptimized(vframe* vf) { - return (vf->is_compiled_frame() && vf->fr().can_be_deoptimized()); - } -}; - -VM_EnterInterpOnlyMode::VM_EnterInterpOnlyMode(JvmtiThreadState *state) - : _state(state) -{ -} - - -void VM_EnterInterpOnlyMode::doit() { - // Set up the current stack depth for later tracking - _state->invalidate_cur_stack_depth(); - - _state->enter_interp_only_mode(); - - JavaThread *thread = _state->get_thread(); - if (thread->has_last_Java_frame()) { - // If running in fullspeed mode, single stepping is implemented - // as follows: first, the interpreter does not dispatch to - // compiled code for threads that have single stepping enabled; - // second, we deoptimize all methods on the thread's stack when - // interpreted-only mode is enabled the first time for a given - // thread (nothing to do if no Java frames yet). - int num_marked = 0; - ResourceMark resMark; - RegisterMap rm(thread, false); - for (vframe* vf = thread->last_java_vframe(&rm); vf; vf = vf->sender()) { - if (can_be_deoptimized(vf)) { - ((compiledVFrame*) vf)->code()->mark_for_deoptimization(); - ++num_marked; + EnterInterpOnlyModeClosure() : HandshakeClosure("EnterInterpOnlyMode") { } + void do_thread(Thread* th) { + JavaThread* jt = (JavaThread*) th; + JvmtiThreadState* state = jt->jvmti_thread_state(); + + // Set up the current stack depth for later tracking + state->invalidate_cur_stack_depth(); + + state->enter_interp_only_mode(); + + if (jt->has_last_Java_frame()) { + // If running in fullspeed mode, single stepping is implemented + // as follows: first, the interpreter does not dispatch to + // compiled code for threads that have single stepping enabled; + // second, we deoptimize all compiled java frames on the thread's stack when + // interpreted-only mode is enabled the first time for a given + // thread (nothing to do if no Java frames yet). + ResourceMark resMark; + for (StackFrameStream fst(jt, false); !fst.is_done(); fst.next()) { + if (fst.current()->can_be_deoptimized()) { + Deoptimization::deoptimize(jt, *fst.current()); + } } } - if (num_marked > 0) { - Deoptimization::deoptimize_all_marked(); - } } -} +}; /////////////////////////////////////////////////////////////// @@ -352,9 +330,12 @@ void VM_ChangeSingleStep::doit() { void JvmtiEventControllerPrivate::enter_interp_only_mode(JvmtiThreadState *state) { EC_TRACE(("[%s] # Entering interpreter only mode", JvmtiTrace::safe_get_thread_name(state->get_thread()))); - - VM_EnterInterpOnlyMode op(state); - VMThread::execute(&op); + EnterInterpOnlyModeClosure hs; + if (SafepointSynchronize::is_at_safepoint()) { + hs.do_thread(state->get_thread()); + } else { + Handshake::execute_direct(&hs, state->get_thread()); + } } diff --git a/src/hotspot/share/prims/jvmtiThreadState.cpp b/src/hotspot/share/prims/jvmtiThreadState.cpp index e9738cc4b48..1cdf009ff65 100644 --- a/src/hotspot/share/prims/jvmtiThreadState.cpp +++ b/src/hotspot/share/prims/jvmtiThreadState.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2003, 2019, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2003, 2020, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -236,9 +236,10 @@ int JvmtiThreadState::count_frames() { void JvmtiThreadState::invalidate_cur_stack_depth() { - guarantee(SafepointSynchronize::is_at_safepoint() || - (JavaThread *)Thread::current() == get_thread(), - "must be current thread or at safepoint"); + assert(SafepointSynchronize::is_at_safepoint() || + (JavaThread *)Thread::current() == get_thread() || + Thread::current() == get_thread()->active_handshaker(), + "bad synchronization with owner thread"); _cur_stack_depth = UNKNOWN_STACK_DEPTH; } diff --git a/src/hotspot/share/runtime/thread.hpp b/src/hotspot/share/runtime/thread.hpp index b33021d63f8..b13b86e796e 100644 --- a/src/hotspot/share/runtime/thread.hpp +++ b/src/hotspot/share/runtime/thread.hpp @@ -2032,9 +2032,11 @@ class JavaThread: public Thread { // Used by the interpreter in fullspeed mode for frame pop, method // entry, method exit and single stepping support. This field is - // only set to non-zero by the VM_EnterInterpOnlyMode VM operation. - // It can be set to zero asynchronously (i.e., without a VM operation - // or a lock) so we have to be very careful. + // only set to non-zero at a safepoint or using a direct handshake + // (see EnterInterpOnlyModeClosure). + // It can be set to zero asynchronously to this threads execution (i.e., without + // safepoint/handshake or a lock) so we have to be very careful. + // Accesses by other threads are synchronized using JvmtiThreadState_lock though. int _interp_only_mode; public: diff --git a/src/hotspot/share/runtime/vmOperations.hpp b/src/hotspot/share/runtime/vmOperations.hpp index e57f75f378a..9b7d79ffc83 100644 --- a/src/hotspot/share/runtime/vmOperations.hpp +++ b/src/hotspot/share/runtime/vmOperations.hpp @@ -90,7 +90,6 @@ template(ChangeBreakpoints) \ template(GetOrSetLocal) \ template(GetCurrentLocation) \ - template(EnterInterpOnlyMode) \ template(ChangeSingleStep) \ template(HeapWalkOperation) \ template(HeapIterateOperation) \