RFR(XS): 8139566 need proper sync for adding default read edges
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Thu Dec 15 07:05:57 UTC 2016
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
More information about the serviceability-dev
mailing list