# HG changeset patch # User egahlin # Date 1544188760 -3600 # Node ID 325c95779368df6c9fe2968a61dc7fb2cbb195b6 # Parent f3d5dcb6924b892ecd28ea9cd2e7841f2027a7c5 8207829: FlightRecorderMXBeanImpl is leaking the first classloader which calls it Reviewed-by: mgronlun diff -r f3d5dcb6924b -r 325c95779368 src/jdk.jfr/share/classes/jdk/jfr/internal/RequestEngine.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 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 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()) { diff -r f3d5dcb6924b -r 325c95779368 src/jdk.jfr/share/classes/jdk/jfr/internal/instrument/JDKEvents.java --- 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) 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()); diff -r f3d5dcb6924b -r 325c95779368 test/jdk/jdk/jfr/jmx/TestFlightRecorderMXBeanLeak.java --- /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); + } + } + } +}