hs25 review request: 8008511 JSR 292: MemberName vmtarget refs to methods must be updated at class redefinition

[David Holmes]

Hi Serguei,

The removal of the NoSafepointVerifier could be problematic. If that 
method is never meant to be on the stack when you block for a safepoint 
then the method is atomic with respect to the safepoint - and so can not 
suffer interference from actions that occur at a safepoint (like class 
unloading (?) ). Now I presume that somewhere from that method you end 
up taking the mutex and so can block at a safepoint (or block on the 
lock and so still allow a safepoint to occur) - which might invalidate 
assumptions the method has about "safepoint atomicity".

You need to understand why the NSV was put there in the first place.

David
------

On 26/03/2013 6:37 PM, serguei.spitsyn at oracle.com wrote:
>
> Please, review the following fix.
> The fix was preliminarily reviewed by Coleen, Christian
> and John but still a final and open jdk review is needed.
> So that, everyone is welcome to review the fix!
>
> This is open webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2013/hotspot/8008511-JVMTI-JSR292.1/
>
> The CR is:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8008511
> https://jbs.oracle.com/bugs/browse/JDK-8008511
>
>
> The problem is that the old version of the bootstrap method is re-invoked
> after a popframe from the bootstrap method execution.
> It is because the MemberName keeps a stale reference to the old method
> version.
>
> The solution (suggested by John Rose) is to lazily create and keep
> up-to-date a MemberNameTable
> which plays a role of MemberName cache assosiated with the InstanceKlass.
> Then, at the Class redefinition, this cache is used to replace the old
> method
> references in the MemberName's with the new method references.
>
> The MemberNameTable is based on the GrowableArray<jweek>.
> A C_HEAP array is allocated at the first call to
> InstanceKlass::add_member_name().
> It is released in the InstanceKlass::release_C_heap_structures().
>
> A global week reference to member name oop is stored in the MemberNameTable.
> It allowed to avoid having the oops_do() in the MemberNameTable.
> Also, the MemberNameTable won't hold member name oops in memory.
>
> The MemberNameTable_lock mutex is added to serialize MemberNameTable's
> updates.
>
> The following No_Safepoint_Verifier has been disabled:
> *  src/share/vm/prims/methodHandles.cpp*:
>
>     799 int MethodHandles::find_MemberNames(Klass* k,
>     . . .
>     803 DEBUG_ONLY(No_Safepoint_Verifier nsv);
>
>      It is probably Ok, but, please, let me know if it is not.
>      The assert related to locking is fired if it is not disabled.
>
> Test coverage: vm.mlvm, nsk.jvmti, nsk.jdi tests on multiple platforms
> (32 vs 64-bit too).
> The testing looks good so far.
>
>
> Thanks,
> Serguei
>
>