diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java index e7b2282cc67..6014bbbf9d2 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -38,6 +38,8 @@ import java.nio.file.NoSuchFileException; import java.nio.file.StandardOpenOption; import java.security.AccessControlContext; import java.security.AccessController; +import java.security.PrivilegedActionException; +import java.security.PrivilegedExceptionAction; import java.time.Duration; import java.time.Instant; import java.time.LocalDateTime; @@ -76,7 +78,7 @@ public final class PlatformRecording implements AutoCloseable { private boolean toDisk = true; private String name; private boolean dumpOnExit; - private SafePath dumpOnExitDirectory = new SafePath("."); + private SafePath dumpDirectory; // Timestamp information private Instant stopTime; private Instant startTime; @@ -89,7 +91,7 @@ public final class PlatformRecording implements AutoCloseable { private TimerTask stopTask; private TimerTask startTask; @SuppressWarnings("removal") - private AccessControlContext noDestinationDumpOnExitAccessControlContext; + private final AccessControlContext dumpDirectoryControlContext; private boolean shouldWriteActiveRecordingEvent = true; private Duration flushInterval = Duration.ofSeconds(1); private long finalStartChunkNanos = Long.MIN_VALUE; @@ -99,11 +101,11 @@ public final class PlatformRecording implements AutoCloseable { PlatformRecording(PlatformRecorder recorder, long id) { // Typically the access control context is taken // when you call dump(Path) or setDestination(Path), - // but if no destination is set and dumpOnExit=true + // but if no destination is set and the filename is auto-generated, // the control context of the recording is taken when the // Recording object is constructed. This works well for // -XX:StartFlightRecording and JFR.dump - this.noDestinationDumpOnExitAccessControlContext = AccessController.getContext(); + this.dumpDirectoryControlContext = AccessController.getContext(); this.id = id; this.recorder = recorder; this.name = String.valueOf(id); @@ -174,7 +176,9 @@ public final class PlatformRecording implements AutoCloseable { newState = getState(); } WriteableUserPath dest = getDestination(); - + if (dest == null && dumpDirectory != null) { + dest = makeDumpPath(); + } if (dest != null) { try { dumpStopped(dest); @@ -191,6 +195,33 @@ public final class PlatformRecording implements AutoCloseable { return true; } + @SuppressWarnings("removal") + public WriteableUserPath makeDumpPath() { + try { + String name = JVMSupport.makeFilename(getRecording()); + return AccessController.doPrivileged(new PrivilegedExceptionAction() { + @Override + public WriteableUserPath run() throws Exception { + SafePath p = dumpDirectory; + if (p == null) { + p = new SafePath("."); + } + return new WriteableUserPath(p.toPath().resolve(name)); + } + }, dumpDirectoryControlContext); + } catch (PrivilegedActionException e) { + Throwable t = e.getCause(); + if (t instanceof SecurityException) { + Logger.log(LogTag.JFR, LogLevel.WARN, "Not allowed to create dump path for recording " + recording.getId() + " on exit."); + } + if (t instanceof IOException) { + Logger.log(LogTag.JFR, LogLevel.WARN, "Could not dump " + recording.getId() + " on exit."); + } + return null; + } + } + + public void scheduleStart(Duration delay) { synchronized (recorder) { ensureOkForSchedule(); @@ -697,11 +728,6 @@ public final class PlatformRecording implements AutoCloseable { destination = null; } - @SuppressWarnings("removal") - public AccessControlContext getNoDestinationDumpOnExitAccessControlContext() { - return noDestinationDumpOnExitAccessControlContext; - } - void setShouldWriteActiveRecordingEvent(boolean shouldWrite) { this.shouldWriteActiveRecordingEvent = shouldWrite; } @@ -828,12 +854,13 @@ public final class PlatformRecording implements AutoCloseable { return result; } - public void setDumpOnExitDirectory(SafePath directory) { - this.dumpOnExitDirectory = directory; - } - - public SafePath getDumpOnExitDirectory() { - return this.dumpOnExitDirectory; + /** + * Sets the dump directory. + *

+ * Only to be used by DCmdStart. + */ + public void setDumpDirectory(SafePath directory) { + this.dumpDirectory = directory; } public void setFlushInterval(Duration interval) { diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/ShutdownHook.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/ShutdownHook.java index 2eabf74404c..afd9ec09bd9 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/ShutdownHook.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/ShutdownHook.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2016, 2024, 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 @@ -65,7 +65,7 @@ final class ShutdownHook implements Runnable { try { WriteableUserPath dest = recording.getDestination(); if (dest == null) { - dest = makeDumpOnExitPath(recording); + dest = recording.makeDumpPath(); recording.setDestination(dest); } if (dest != null) { @@ -78,29 +78,6 @@ final class ShutdownHook implements Runnable { } } - @SuppressWarnings("removal") - private WriteableUserPath makeDumpOnExitPath(PlatformRecording recording) { - try { - String name = JVMSupport.makeFilename(recording.getRecording()); - AccessControlContext acc = recording.getNoDestinationDumpOnExitAccessControlContext(); - return AccessController.doPrivileged(new PrivilegedExceptionAction() { - @Override - public WriteableUserPath run() throws Exception { - return new WriteableUserPath(recording.getDumpOnExitDirectory().toPath().resolve(name)); - } - }, acc); - } catch (PrivilegedActionException e) { - Throwable t = e.getCause(); - if (t instanceof SecurityException) { - Logger.log(LogTag.JFR, LogLevel.WARN, "Not allowed to create dump path for recording " + recording.getId() + " on exit."); - } - if (t instanceof IOException) { - Logger.log(LogTag.JFR, LogLevel.WARN, "Could not dump " + recording.getId() + " on exit."); - } - return null; - } - } - static final class ExceptionHandler implements Thread.UncaughtExceptionHandler { @Override public void uncaughtException(Thread t, Throwable e) { diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/DCmdStart.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/DCmdStart.java index aa7165cb975..a7e423e549a 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/DCmdStart.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/DCmdStart.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2024, 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 @@ -29,6 +29,8 @@ import java.nio.file.Files; import java.nio.file.InvalidPathException; import java.nio.file.Path; import java.nio.file.Paths; +import java.security.AccessControlContext; +import java.security.AccessController; import java.text.ParseException; import java.time.Duration; import java.util.HashSet; @@ -165,11 +167,12 @@ final class DCmdStart extends AbstractDCmd { dumpOnExit = Boolean.TRUE; } Path p = Paths.get(path); - if (Files.isDirectory(p) && Boolean.TRUE.equals(dumpOnExit)) { + if (Files.isDirectory(p)) { // Decide destination filename at dump time // Purposely avoid generating filename in Recording#setDestination due to // security concerns - PrivateAccess.getInstance().getPlatformRecording(recording).setDumpOnExitDirectory(new SafePath(p)); + PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording); + pr.setDumpDirectory(new SafePath(p)); } else { safePath = resolvePath(recording, path); recording.setDestination(safePath.toPath()); diff --git a/test/jdk/jdk/jfr/jcmd/TestJcmdStartGeneratedFilename.java b/test/jdk/jdk/jfr/jcmd/TestJcmdStartGeneratedFilename.java new file mode 100644 index 00000000000..d3e4dd7b2c2 --- /dev/null +++ b/test/jdk/jdk/jfr/jcmd/TestJcmdStartGeneratedFilename.java @@ -0,0 +1,70 @@ +/* + * Copyright (c) 2024, 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. + * + * 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.jcmd; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.concurrent.CountDownLatch; + +import jdk.jfr.FlightRecorder; +import jdk.jfr.FlightRecorderListener; +import jdk.jfr.Recording; +import jdk.jfr.RecordingState; + +import jdk.test.lib.process.OutputAnalyzer; + +/** + * @test + * @summary Verify that a filename is generated + * @key jfr + * @requires vm.hasJFR + * @library /test/lib /test/jdk + * @run main/othervm jdk.jfr.jcmd.TestJcmdStartGeneratedFilename + */ +public class TestJcmdStartGeneratedFilename { + + public static void main(String[] args) throws Exception { + CountDownLatch recordingClosed = new CountDownLatch(1); + FlightRecorder.addListener(new FlightRecorderListener() { + public void recordingStateChanged(Recording recording) { + if (recording.getState() == RecordingState.CLOSED) { + recordingClosed.countDown(); + } + } + }); + Path directory = Paths.get(".", "recordings"); + Files.createDirectories(directory); + JcmdHelper.jcmd("JFR.start", "duration=1s", "filename=" + directory); + recordingClosed.await(); + for (Path path : Files.list(directory).toList()) { + String file = path.toString(); + System.out.println("Found file: " + file); + if (file.endsWith(".jfr") && file.contains("hotspot-")) { + return; + } + } + throw new Exception("Expected dump file on the format hotspot-...jfr"); + } +}