RFR(XS): 8139566 need proper sync for adding default read edges

[serguei.spitsyn at oracle.com]

On 12/14/16 02:01, serguei.spitsyn at oracle.com wrote:
> Please, review a fix for the bug:
> https://bugs.openjdk.java.net/browse/JDK-8139566
>
>
> Webrev:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8139566-SyncDefReadEdges.hs1/ 
>

The webrev where a review comment from Lois has been resolved:
http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8139566-SyncDefReadEdges.hs2/


Thanks,
Serguei


>
>
> There are already two positive (preliminary) reviews from Harold and 
> Lois.
>
>
> Summary:
>   This is a follow-up issue after the fixes for:
> https://bugs.openjdk.java.net/browse/JDK-8134950
> https://bugs.openjdk.java.net/browse/JDK-8072745
>
>   The webrevs for the issues above are:
> http://cr.openjdk.java.net/~sspitsyn/webrevs/2015/hotspot/8134950-JVMTI-jake-ana2.3/ 
>
> http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2015/hotspot/8072745-JVMTI-jake-ana1.5/ 
>
>
>   The issue is that there can be a race with another thread that 
> checks the
>   ModuleEntry::has_default_read_edges() and goes with its 
> redefinition/retransformation
>   while the upcall to the method 
> jdk.internal.module.Modules.transformedByAgent()
>   (that adds the default read edges to bootstrap and app class 
> loaders) is still in progress.
>   The instrumented code can execute while the upcall to 
> transformedByAgent has not been completed yet.
>   Please, refer to the bug description to get more details.
>
>   The fix is to check the ModuleEntry::has_default_read_edges() in 
> such a case.
>   The check is added to the ModuleEntry::can_read as it is used in all
>   class resolution cases, for instance:
>
>     LinkResolver::check_klass_accessability() ->
>       Reflection::verify_class_access() ->
>         ModuleEntry::can_read()
>
>     ClassFileParser::check_super_class_access() ->
>       Reflection::verify_class_access() ->
>         ModuleEntry::can_read()
>
>     ClassFileParser::check_super_interface_access() ->
>       Reflection::verify_class_access() ->
>         ModuleEntry::can_read()
>
>   Note that the ModuleEntry::_has_default_read_edges bit was added as 
> a part
>   of the 8144950 to avoid a bad recursion.
>
>   One question is:
>   Q1: Can we remove the upcall to the 
> jdk.internal.module.Modules.transformedByAgent()
>       as it would not be needed anymore? The implementation could me 
> simplified.
>
>   A1: I think, the real read edges established via an upcall to the 
> Java runtime are still
>      needed for the Java level based mechanisms that do not use the 
> ModuleEntry::can_read().
>      These mechanisms, probably, can tolerate that the "adding to a 
> module default read edges"
>      signal arrives with some latency.
>
>
> Thanks,
> Serguei