8323425: JFR: Auto-generated filename doesn't work with time-limited recording

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2024-01-12 13:12:37 +00:00
parent 68c4286026
commit be900f1253
4 changed files with 122 additions and 45 deletions

View File

@ -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<WriteableUserPath>() {
@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.
* <p>
* Only to be used by DCmdStart.
*/
public void setDumpDirectory(SafePath directory) {
this.dumpDirectory = directory;
}
public void setFlushInterval(Duration interval) {

View File

@ -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<WriteableUserPath>() {
@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) {

View File

@ -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());

View File

@ -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");
}
}