Serguei, Thank you for the explanation. Looks good for me.
-Dmitry On 2013-11-27 13:31, [email protected] wrote: > Dmitry, > > Thank you for reviewing! > > On 11/27/13 12:08 AM, Dmitry Samersoff wrote: >> Serguei, >> >> Is it better to just convert >> assert at ll. 273 into if (vf != NULL) ... ? > > Your suggestion does not help as we hit another assert in the call to > last_java_vframe(): > javaVFrame* vf = _thread->last_java_vframe(&rm); > > This is the original assert that was reported in the bug: > > # Internal Error > (/tmp/jprt/P1/150447.amurillo/s/src/cpu/sparc/vm/frame_sparc.cpp:718), > pid=29913, tid=5 > # guarantee(Thread::current() == (Thread*)thread) failed: only current > thread can flush its registers > > The stack is: > Stack: [0xffffffff06e00000,0xffffffff06f00000], sp=0xffffffff06efea60, > free space=1018k > Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, > C=native code) > V [libjvm.so+0x12379ac] void VMError::report_and_die()+0x714;; > __1cHVMErrorOreport_and_die6M_v_+0x714 > V [libjvm.so+0x71daf8] void report_vm_error(const char*,int,const > char*,const char*)+0x78;; __1cPreport_vm_error6Fpkci11_v_+0x78 > V [libjvm.so+0x7e76bc] void > JavaFrameAnchor::make_walkable(JavaThread*)+0x68;; > __1cPJavaFrameAnchorNmake_walkable6MpnKJavaThread__v_+0x68 > *V [libjvm.so+0x1189358] > javaVFrame*JavaThread::last_java_vframe(RegisterMap*)+0x50;; > __1cKJavaThreadQlast_java_vframe6MpnLRegisterMap__pnKjavaVFrame__+0x50* > V [libjvm.so+0xc6b3c8] void VM_GetCurrentLocation::doit()+0xd0;; > __1cVVM_GetCurrentLocationEdoit6M_v_+0xd0 > V [libjvm.so+0x1267a50] void VM_Operation::evaluate()+0xf8;; > __1cMVM_OperationIevaluate6M_v_+0xf8 > V [libjvm.so+0x1263994] void > VMThread::evaluate_operation(VM_Operation*)+0x254;; > __1cIVMThreadSevaluate_operation6MpnMVM_Operation__v_+0x254 > V [libjvm.so+0x1264454] void VMThread::loop()+0x6a4;; > __1cIVMThreadEloop6M_v_+0x6a4 > V [libjvm.so+0x12633fc] void VMThread::run()+0xe4;; > __1cIVMThreadDrun6M_v_+0xe4 > V [libjvm.so+0xf183b8] java_start+0x258;; java_start+0x258 > > > Thanks, > Serguei > >> >> -Dmitry >> >> >> >> On 2013-11-27 05:34, [email protected] wrote: >>> Please, review the fix for: >>> https://bugs.openjdk.java.net/browse/JDK-8028126 >>> >>> Open webrev: >>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8028126-JVMTI-HS101.1/ >>> >>> >>> Summary: >>> This is a fix for a possible race condition between the >>> VMOp_GetCurrentLocation >>> reaching a safepoint and target debuggee thread exiting from Java >>> execution. >>> The fix is to recheck the existence of the last Java frame at a safepoint >>> and clean the thread current location if the thread has been already >>> exited from Java. >>> >>> I'm suggesting to fix this in hs25/JDK 8. >>> It is important to fix as it is a P2 bug and the risk of fixing it is >>> low. >>> But need reviewers to share opinions on this. >>> I'll add the 8-critical-request label if reviewers agree with the above. >>> >>> >>> Testing: >>> The test nsk/jvmti/scenarios/hotswap/HS101/hs101t006 that was >>> originally failed. >>> In progress: nsk.jvmti, nsk.jdi, nsk.jdwp >>> >>> >>> Thanks, >>> Serguei >> > -- Dmitry Samersoff Oracle Java development team, Saint Petersburg, Russia * I would love to change the world, but they won't give me the sources.
