8207829: FlightRecorderMXBeanImpl is leaking the first classloader which calls it
authoregahlin
Fri, 07 Dec 2018 14:19:20 +0100
changeset 52899 325c95779368
parent 52898 f3d5dcb6924b
child 52900 2998e6d76879
child 57063 1fa5c73d3c5a
8207829: FlightRecorderMXBeanImpl is leaking the first classloader which calls it Reviewed-by: mgronlun
src/jdk.jfr/share/classes/jdk/jfr/internal/RequestEngine.java
src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java
test/jdk/jdk/jfr/jmx/TestFlightRecorderMXBeanLeak.java
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/RequestEngine.java	Fri Dec 07 08:16:50 2018 -0500
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/RequestEngine.java	Fri Dec 07 14:19:20 2018 +0100
@@ -34,6 +34,8 @@
 import java.util.Objects;
 import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Predicate;
+import jdk.jfr.Event;
+import jdk.jfr.EventType;
 
 public final class RequestEngine {
 
@@ -60,7 +62,11 @@
         private void execute() {
             try {
                 if (accessControllerContext == null) { // native
-                    jvm.emitEvent(type.getId(), JVM.counterTime(), 0);
+                    if (type.isJDK()) {
+                        hook.run();
+                    } else {
+                        jvm.emitEvent(type.getId(), JVM.counterTime(), 0);
+                    }
                     Logger.log(LogTag.JFR_SYSTEM_EVENT, LogLevel.DEBUG, ()-> "Executed periodic hook for " + type.getLogName());
                 } else {
                     executeSecure();
@@ -91,11 +97,12 @@
     private final static List<RequestHook> entries = new CopyOnWriteArrayList<>();
     private static long lastTimeMillis;
 
-    // Insertion takes O(2*n), could be O(1) with HashMap, but
-    // thinking is that CopyOnWriteArrayList is faster
-    // to iterate over, which will happen more over time.
     public static void addHook(AccessControlContext acc, PlatformEventType type, Runnable hook) {
         Objects.requireNonNull(acc);
+        addHookInternal(acc, type, hook);
+    }
+
+    private static void addHookInternal(AccessControlContext acc, PlatformEventType type, Runnable hook) {
         RequestHook he = new RequestHook(acc, type, hook);
         for (RequestHook e : entries) {
             if (e.hook == hook) {
@@ -103,10 +110,24 @@
             }
         }
         he.type.setEventHook(true);
+        // Insertion takes O(2*n), could be O(1) with HashMap, but
+        // thinking is that CopyOnWriteArrayList is faster
+        // to iterate over, which will happen more over time.
         entries.add(he);
         logHook("Added", type);
     }
 
+    public static void addTrustedJDKHook(Class<? extends Event> eventClass, Runnable runnable) {
+        if (eventClass.getClassLoader() != null) {
+            throw new SecurityException("Hook can only be registered for event classes that are loaded by the bootstrap class loader");
+        }
+        if (runnable.getClass().getClassLoader() != null) {
+            throw new SecurityException("Runnable hook class must be loaded by the bootstrap class loader");
+        }
+        EventType eType = MetadataRepository.getInstance().getEventType(eventClass);
+        PlatformEventType pType = PrivateAccess.getInstance().getPlatformEventType(eType);
+        addHookInternal(null, pType, runnable);
+    }
 
     private static void logHook(String action, PlatformEventType type) {
         if (type.isJDK() || type.isJVM()) {
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java	Fri Dec 07 08:16:50 2018 -0500
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java	Fri Dec 07 14:19:20 2018 +0100
@@ -29,7 +29,6 @@
 import java.util.List;
 
 import jdk.jfr.Event;
-import jdk.jfr.FlightRecorder;
 import jdk.jfr.events.ActiveRecordingEvent;
 import jdk.jfr.events.ActiveSettingEvent;
 import jdk.jfr.events.ErrorThrownEvent;
@@ -50,7 +49,6 @@
 import jdk.jfr.internal.Logger;
 import jdk.jfr.internal.RequestEngine;
 import jdk.jfr.internal.SecuritySupport;
-import jdk.jfr.internal.Utils;
 
 public final class JDKEvents {
 
@@ -105,7 +103,7 @@
                     SecuritySupport.registerEvent((Class<? extends Event>) eventClass);
                 }
                 initializationTriggered = true;
-                FlightRecorder.addPeriodicEvent(ExceptionStatisticsEvent.class, emitExceptionStatistics);
+                RequestEngine.addTrustedJDKHook(ExceptionStatisticsEvent.class, emitExceptionStatistics);
             }
         } catch (Exception e) {
             Logger.log(LogTag.JFR_SYSTEM, LogLevel.WARN, "Could not initialize JDK events. " + e.getMessage());
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/jdk/jfr/jmx/TestFlightRecorderMXBeanLeak.java	Fri Dec 07 14:19:20 2018 +0100
@@ -0,0 +1,95 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package jdk.jfr.jmx;
+
+import java.lang.management.ManagementFactory;
+import java.net.URL;
+import java.net.URLClassLoader;
+
+import javax.management.MBeanAttributeInfo;
+import javax.management.MBeanInfo;
+import javax.management.MBeanServer;
+import javax.management.ObjectName;
+
+import jdk.jfr.Recording;
+import jdk.jfr.consumer.RecordedClassLoader;
+import jdk.jfr.consumer.RecordedEvent;
+import jdk.management.jfr.FlightRecorderMXBean;
+import jdk.test.lib.jfr.Events;
+
+/**
+ * @test
+ * @key jfr
+ * @summary Verifies that attributes in FlightRecorderMXBean can be inspected
+ *          without causing a memory leak.
+ * @requires vm.hasJFR
+ * @library /test/lib /test/jdk
+ * @run main/othervm jdk.jfr.jmx.TestFlightRecorderMXBeanLeak
+ */
+public class TestFlightRecorderMXBeanLeak {
+
+    private static final String CLASS_LOADER_NAME = "Test Leak";
+    private static final String TEST_CLASS = "jdk.jfr.jmx.TestFlightRecorderMXBeanLeak$FlightRecorderMXBeanLoader";
+
+    public static void main(String[] args) throws Exception {
+        URL url = FlightRecorderMXBeanLoader.class.getProtectionDomain().getCodeSource().getLocation();
+        URLClassLoader loader = new URLClassLoader(CLASS_LOADER_NAME, new URL[] { url }, null);
+        Class<?> clazz = Class.forName(TEST_CLASS, true, loader);
+        clazz.newInstance();
+        loader.close();
+        clazz = null;
+        loader = null;
+        System.gc();
+        System.gc();
+
+        try (Recording r = new Recording()) {
+            r.enable("jdk.ClassLoaderStatistics").with("period", "endChunk");
+            r.start();
+            r.stop();
+            for (RecordedEvent e : Events.fromRecording(r)) {
+                RecordedClassLoader o = e.getValue("classLoader");
+                if (o != null) {
+                    System.out.println("Class loader: type=" + o.getType().getName() + " name=" + o.getName());
+                    if (CLASS_LOADER_NAME.equals(o.getName())) {
+                        throw new Exception("Memory Leak. Class loader '" + CLASS_LOADER_NAME + "' should not be on the heap!");
+                    }
+                }
+            }
+        }
+    }
+
+    public static class FlightRecorderMXBeanLoader {
+        public FlightRecorderMXBeanLoader() throws Exception {
+            ObjectName objectName = new ObjectName(FlightRecorderMXBean.MXBEAN_NAME);
+            MBeanServer platformMBeanServer = ManagementFactory.getPlatformMBeanServer();
+            MBeanInfo mBeanInfo = platformMBeanServer.getMBeanInfo(objectName);
+            System.out.println("Inspecting FlightRecorderMXBeann:");
+            for (MBeanAttributeInfo attributeInfo : mBeanInfo.getAttributes()) {
+                Object value = platformMBeanServer.getAttribute(objectName, attributeInfo.getName());
+                System.out.println(attributeInfo.getName() + " = " + value);
+            }
+        }
+    }
+}