8207829: FlightRecorderMXBeanImpl is leaking the first classloader which calls it
Reviewed-by: mgronlun
--- 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);
+ }
+ }
+ }
+}