From bdaf8c260e7e880145df8ac8c09b19570c6c37c6 Mon Sep 17 00:00:00 2001 From: Jie Kang Date: Thu, 29 Jul 2021 13:37:42 -0400 Subject: [PATCH 1/3] Fix reflection registration for JFR eventHandler fields Closes #3608 --- .../oracle/svm/jfr/JfrEventSubstitution.java | 4 -- .../src/com/oracle/svm/jfr/JfrFeature.java | 38 +++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java index 9e7479825b2e..2cd2e68843e1 100644 --- a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java +++ b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java @@ -34,13 +34,11 @@ import org.graalvm.compiler.serviceprovider.GraalUnsafeAccess; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; -import org.graalvm.nativeimage.hosted.RuntimeReflection; import com.oracle.graal.pointsto.infrastructure.OriginalClassProvider; import com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor; import com.oracle.svm.core.util.VMError; import com.oracle.svm.hosted.c.GraalAccess; -import com.oracle.svm.util.ReflectionUtil; import jdk.jfr.Event; import jdk.jfr.internal.EventWriter; @@ -79,8 +77,6 @@ private static Boolean initEventClass(ResolvedJavaType eventType) throws Runtime SecuritySupport.registerEvent(newEventClass); JfrJavaEvents.registerEventClass(newEventClass); JVM.getJVM().retransformClasses(new Class[]{newEventClass}); - Field field = ReflectionUtil.lookupField(newEventClass, "eventHandler"); // EventInstrumentation.FIELD_EVENT_HANDLER - RuntimeReflection.register(field); return Boolean.TRUE; } catch (Throwable ex) { throw VMError.shouldNotReachHere(ex); diff --git a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java index ab7903223668..f103d0dc39f9 100644 --- a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java +++ b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java @@ -24,10 +24,17 @@ */ package com.oracle.svm.jfr; +//Checkstyle: allow reflection + +import java.lang.reflect.Field; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import com.oracle.svm.core.util.VMError; +import jdk.vm.ci.meta.ResolvedJavaType; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; @@ -55,6 +62,7 @@ import jdk.jfr.internal.JVM; import jdk.jfr.internal.jfc.JFC; import jdk.vm.ci.meta.MetaAccessProvider; +import org.graalvm.nativeimage.hosted.RuntimeReflection; /** * Provides basic JFR support. As this support is both platform-dependent and JDK-specific, the @@ -97,6 +105,9 @@ @Platforms({Platform.LINUX.class, Platform.DARWIN.class}) @AutomaticFeature public class JfrFeature implements Feature { + + private final ConcurrentHashMap fieldRegistration = new ConcurrentHashMap<>(); + @Override public boolean isInConfiguration(IsInConfigurationAccess access) { return JfrEnabled.get(); @@ -163,4 +174,31 @@ public void beforeCompilation(BeforeCompilationAccess a) { JfrTraceId.assign(clazz, hub.getTypeID() + 1); } } + + @Override + public void duringAnalysis(DuringAnalysisAccess access) { + Class eventClass = access.findClassByName("jdk.internal.event.Event"); + if (eventClass != null && access.isReachable(eventClass)) { + Set> s = access.reachableSubtypes(eventClass); + for (Class c : s) { + if (c.getCanonicalName().equals("jdk.jfr.Event") + || c.getCanonicalName().equals("jdk.internal.event.Event") + || c.getCanonicalName().equals("jdk.jfr.events.AbstractJDKEvent")) { + continue; + } + fieldRegistration.computeIfAbsent(c, this::registerEventHandler); + + } + } + } + + private boolean registerEventHandler(Class c) { + try { + Field f = c.getDeclaredField("eventHandler"); + RuntimeReflection.register(f); + } catch (Exception e) { + throw VMError.shouldNotReachHere("Unable to register eventHandler for: " + c.getCanonicalName(), e); + } + return Boolean.TRUE; + } } From 74964161962d4111a715f95f6d475315e59e691b Mon Sep 17 00:00:00 2001 From: Jie Kang Date: Mon, 9 Aug 2021 14:38:03 -0400 Subject: [PATCH 2/3] Add comment explaining registration of eventHandler --- .../src/com/oracle/svm/jfr/JfrEventSubstitution.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java index 2cd2e68843e1..b44f203846d1 100644 --- a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java +++ b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrEventSubstitution.java @@ -76,6 +76,8 @@ private static Boolean initEventClass(ResolvedJavaType eventType) throws Runtime eventType.initialize(); SecuritySupport.registerEvent(newEventClass); JfrJavaEvents.registerEventClass(newEventClass); + // the reflection registration for the event handler field is delayed to the JfrFeature + // duringAnalysis callback so it does not not race/interfere with other retransforms JVM.getJVM().retransformClasses(new Class[]{newEventClass}); return Boolean.TRUE; } catch (Throwable ex) { From e7476d8f4d66aecc8ae15b4ce9f705d2146bbcd9 Mon Sep 17 00:00:00 2001 From: Jie Kang Date: Wed, 11 Aug 2021 21:11:25 -0400 Subject: [PATCH 3/3] Add comment for getting canonical name. Remove redundant deduplication structure --- .../src/com/oracle/svm/jfr/JfrFeature.java | 23 ++++++------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java index f103d0dc39f9..1a719a2b9db6 100644 --- a/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java +++ b/substratevm/src/com.oracle.svm.jfr/src/com/oracle/svm/jfr/JfrFeature.java @@ -31,10 +31,8 @@ import java.util.Collections; import java.util.List; import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; import com.oracle.svm.core.util.VMError; -import jdk.vm.ci.meta.ResolvedJavaType; import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.nativeimage.Platform; import org.graalvm.nativeimage.Platforms; @@ -106,8 +104,6 @@ @AutomaticFeature public class JfrFeature implements Feature { - private final ConcurrentHashMap fieldRegistration = new ConcurrentHashMap<>(); - @Override public boolean isInConfiguration(IsInConfigurationAccess access) { return JfrEnabled.get(); @@ -181,24 +177,19 @@ public void duringAnalysis(DuringAnalysisAccess access) { if (eventClass != null && access.isReachable(eventClass)) { Set> s = access.reachableSubtypes(eventClass); for (Class c : s) { + // Use canonical name for package private AbstractJDKEvent if (c.getCanonicalName().equals("jdk.jfr.Event") || c.getCanonicalName().equals("jdk.internal.event.Event") || c.getCanonicalName().equals("jdk.jfr.events.AbstractJDKEvent")) { continue; } - fieldRegistration.computeIfAbsent(c, this::registerEventHandler); - + try { + Field f = c.getDeclaredField("eventHandler"); + RuntimeReflection.register(f); + } catch (Exception e) { + throw VMError.shouldNotReachHere("Unable to register eventHandler for: " + c.getCanonicalName(), e); + } } } } - - private boolean registerEventHandler(Class c) { - try { - Field f = c.getDeclaredField("eventHandler"); - RuntimeReflection.register(f); - } catch (Exception e) { - throw VMError.shouldNotReachHere("Unable to register eventHandler for: " + c.getCanonicalName(), e); - } - return Boolean.TRUE; - } }