review (S) for 6956931: assert(SafepointSynchronize::is_at_safepoint()) failed: must be executed at a safepoint

[Daniel D. Daugherty]

src/share/vm/code/nmethod.cpp
    Looks like the old code (1344-1358) had the possibility of a
    double event post for both JVM/TI and dtrace. The newly
    refactored code doesn't have that (possible) issue. Nice fix.

    I'm not aware of any open bugs complaining about double
    CompiledMethodUnload events or dtrace probes, but if there are
    I think you've fixed those too.

src/share/vm/prims/jvmtiExport.hpp
    internal_post_compiled_method_unload() should be
    post_compiled_method_unload_internal() if you want to follow
    the existing style in jvmtiExport.

src/share/vm/prims/jvmtiExport.cpp
    Same internal_post_compiled_method_unload() naming issue

Dan


On 6/2/2010 10:51 AM, Tom Rodriguez wrote:
> http://cr.openjdk.java.net/~never/6956931
>
> 6956931: assert(SafepointSynchronize::is_at_safepoint()) failed: must be executed at a safepoint
> Reviewed-by:
>
> SSince the sweeper no longer runs at a safepoint the compiled method
> unload logic doesn't always run at a safepoint so we need a new
> synchronous notification path that's equivalent to the
> compiled_method_load logic.  I refactored the code a bit to share the
> core notification logic and I refactored the notification in nmethod
> to remove duplication.  Tested with NSK jvmti,jdb,jdi,hprof suites.