Skip to content

Commit be900f1

Browse files
egahlinegahlin
committed
8323425: JFR: Auto-generated filename doesn't work with time-limited recording
Reviewed-by: mgronlun
1 parent 68c4286 commit be900f1

4 files changed

Lines changed: 122 additions & 45 deletions

File tree

‎src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java‎

Lines changed: 44 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -38,6 +38,8 @@
3838
import java.nio.file.StandardOpenOption;
3939
import java.security.AccessControlContext;
4040
import java.security.AccessController;
41+
import java.security.PrivilegedActionException;
42+
import java.security.PrivilegedExceptionAction;
4143
import java.time.Duration;
4244
import java.time.Instant;
4345
import java.time.LocalDateTime;
@@ -76,7 +78,7 @@ public final class PlatformRecording implements AutoCloseable {
7678
private boolean toDisk = true;
7779
private String name;
7880
private boolean dumpOnExit;
79-
private SafePath dumpOnExitDirectory = new SafePath(".");
81+
private SafePath dumpDirectory;
8082
// Timestamp information
8183
private Instant stopTime;
8284
private Instant startTime;
@@ -89,7 +91,7 @@ public final class PlatformRecording implements AutoCloseable {
8991
private TimerTask stopTask;
9092
private TimerTask startTask;
9193
@SuppressWarnings("removal")
92-
private AccessControlContext noDestinationDumpOnExitAccessControlContext;
94+
private final AccessControlContext dumpDirectoryControlContext;
9395
private boolean shouldWriteActiveRecordingEvent = true;
9496
private Duration flushInterval = Duration.ofSeconds(1);
9597
private long finalStartChunkNanos = Long.MIN_VALUE;
@@ -99,11 +101,11 @@ public final class PlatformRecording implements AutoCloseable {
99101
PlatformRecording(PlatformRecorder recorder, long id) {
100102
// Typically the access control context is taken
101103
// when you call dump(Path) or setDestination(Path),
102-
// but if no destination is set and dumpOnExit=true
104+
// but if no destination is set and the filename is auto-generated,
103105
// the control context of the recording is taken when the
104106
// Recording object is constructed. This works well for
105107
// -XX:StartFlightRecording and JFR.dump
106-
this.noDestinationDumpOnExitAccessControlContext = AccessController.getContext();
108+
this.dumpDirectoryControlContext = AccessController.getContext();
107109
this.id = id;
108110
this.recorder = recorder;
109111
this.name = String.valueOf(id);
@@ -174,7 +176,9 @@ public boolean stop(String reason) {
174176
newState = getState();
175177
}
176178
WriteableUserPath dest = getDestination();
177-
179+
if (dest == null && dumpDirectory != null) {
180+
dest = makeDumpPath();
181+
}
178182
if (dest != null) {
179183
try {
180184
dumpStopped(dest);
@@ -191,6 +195,33 @@ public boolean stop(String reason) {
191195
return true;
192196
}
193197

198+
@SuppressWarnings("removal")
199+
public WriteableUserPath makeDumpPath() {
200+
try {
201+
String name = JVMSupport.makeFilename(getRecording());
202+
return AccessController.doPrivileged(new PrivilegedExceptionAction<WriteableUserPath>() {
203+
@Override
204+
public WriteableUserPath run() throws Exception {
205+
SafePath p = dumpDirectory;
206+
if (p == null) {
207+
p = new SafePath(".");
208+
}
209+
return new WriteableUserPath(p.toPath().resolve(name));
210+
}
211+
}, dumpDirectoryControlContext);
212+
} catch (PrivilegedActionException e) {
213+
Throwable t = e.getCause();
214+
if (t instanceof SecurityException) {
215+
Logger.log(LogTag.JFR, LogLevel.WARN, "Not allowed to create dump path for recording " + recording.getId() + " on exit.");
216+
}
217+
if (t instanceof IOException) {
218+
Logger.log(LogTag.JFR, LogLevel.WARN, "Could not dump " + recording.getId() + " on exit.");
219+
}
220+
return null;
221+
}
222+
}
223+
224+
194225
public void scheduleStart(Duration delay) {
195226
synchronized (recorder) {
196227
ensureOkForSchedule();
@@ -697,11 +728,6 @@ void clearDestination() {
697728
destination = null;
698729
}
699730

700-
@SuppressWarnings("removal")
701-
public AccessControlContext getNoDestinationDumpOnExitAccessControlContext() {
702-
return noDestinationDumpOnExitAccessControlContext;
703-
}
704-
705731
void setShouldWriteActiveRecordingEvent(boolean shouldWrite) {
706732
this.shouldWriteActiveRecordingEvent = shouldWrite;
707733
}
@@ -828,12 +854,13 @@ private static List<RepositoryChunk> reduceFromEnd(Long maxSize, List<Repository
828854
return result;
829855
}
830856

831-
public void setDumpOnExitDirectory(SafePath directory) {
832-
this.dumpOnExitDirectory = directory;
833-
}
834-
835-
public SafePath getDumpOnExitDirectory() {
836-
return this.dumpOnExitDirectory;
857+
/**
858+
* Sets the dump directory.
859+
* <p>
860+
* Only to be used by DCmdStart.
861+
*/
862+
public void setDumpDirectory(SafePath directory) {
863+
this.dumpDirectory = directory;
837864
}
838865

839866
public void setFlushInterval(Duration interval) {

‎src/jdk.jfr/share/classes/jdk/jfr/internal/ShutdownHook.java‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2016, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -65,7 +65,7 @@ private void dump(PlatformRecording recording) {
6565
try {
6666
WriteableUserPath dest = recording.getDestination();
6767
if (dest == null) {
68-
dest = makeDumpOnExitPath(recording);
68+
dest = recording.makeDumpPath();
6969
recording.setDestination(dest);
7070
}
7171
if (dest != null) {
@@ -78,29 +78,6 @@ private void dump(PlatformRecording recording) {
7878
}
7979
}
8080

81-
@SuppressWarnings("removal")
82-
private WriteableUserPath makeDumpOnExitPath(PlatformRecording recording) {
83-
try {
84-
String name = JVMSupport.makeFilename(recording.getRecording());
85-
AccessControlContext acc = recording.getNoDestinationDumpOnExitAccessControlContext();
86-
return AccessController.doPrivileged(new PrivilegedExceptionAction<WriteableUserPath>() {
87-
@Override
88-
public WriteableUserPath run() throws Exception {
89-
return new WriteableUserPath(recording.getDumpOnExitDirectory().toPath().resolve(name));
90-
}
91-
}, acc);
92-
} catch (PrivilegedActionException e) {
93-
Throwable t = e.getCause();
94-
if (t instanceof SecurityException) {
95-
Logger.log(LogTag.JFR, LogLevel.WARN, "Not allowed to create dump path for recording " + recording.getId() + " on exit.");
96-
}
97-
if (t instanceof IOException) {
98-
Logger.log(LogTag.JFR, LogLevel.WARN, "Could not dump " + recording.getId() + " on exit.");
99-
}
100-
return null;
101-
}
102-
}
103-
10481
static final class ExceptionHandler implements Thread.UncaughtExceptionHandler {
10582
@Override
10683
public void uncaughtException(Thread t, Throwable e) {

‎src/jdk.jfr/share/classes/jdk/jfr/internal/dcmd/DCmdStart.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2012, 2023, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2012, 2024, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -29,6 +29,8 @@
2929
import java.nio.file.InvalidPathException;
3030
import java.nio.file.Path;
3131
import java.nio.file.Paths;
32+
import java.security.AccessControlContext;
33+
import java.security.AccessController;
3234
import java.text.ParseException;
3335
import java.time.Duration;
3436
import java.util.HashSet;
@@ -165,11 +167,12 @@ public void execute(ArgumentParser parser) throws DCmdException {
165167
dumpOnExit = Boolean.TRUE;
166168
}
167169
Path p = Paths.get(path);
168-
if (Files.isDirectory(p) && Boolean.TRUE.equals(dumpOnExit)) {
170+
if (Files.isDirectory(p)) {
169171
// Decide destination filename at dump time
170172
// Purposely avoid generating filename in Recording#setDestination due to
171173
// security concerns
172-
PrivateAccess.getInstance().getPlatformRecording(recording).setDumpOnExitDirectory(new SafePath(p));
174+
PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
175+
pr.setDumpDirectory(new SafePath(p));
173176
} else {
174177
safePath = resolvePath(recording, path);
175178
recording.setDestination(safePath.toPath());

‎test/jdk/jdk/jfr/jcmd/TestJcmdStartGeneratedFilename.java‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/*
2+
* Copyright (c) 2024, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
package jdk.jfr.jcmd;
25+
26+
import java.nio.file.Files;
27+
import java.nio.file.Path;
28+
import java.nio.file.Paths;
29+
import java.util.concurrent.CountDownLatch;
30+
31+
import jdk.jfr.FlightRecorder;
32+
import jdk.jfr.FlightRecorderListener;
33+
import jdk.jfr.Recording;
34+
import jdk.jfr.RecordingState;
35+
36+
import jdk.test.lib.process.OutputAnalyzer;
37+
38+
/**
39+
* @test
40+
* @summary Verify that a filename is generated
41+
* @key jfr
42+
* @requires vm.hasJFR
43+
* @library /test/lib /test/jdk
44+
* @run main/othervm jdk.jfr.jcmd.TestJcmdStartGeneratedFilename
45+
*/
46+
public class TestJcmdStartGeneratedFilename {
47+
48+
public static void main(String[] args) throws Exception {
49+
CountDownLatch recordingClosed = new CountDownLatch(1);
50+
FlightRecorder.addListener(new FlightRecorderListener() {
51+
public void recordingStateChanged(Recording recording) {
52+
if (recording.getState() == RecordingState.CLOSED) {
53+
recordingClosed.countDown();
54+
}
55+
}
56+
});
57+
Path directory = Paths.get(".", "recordings");
58+
Files.createDirectories(directory);
59+
JcmdHelper.jcmd("JFR.start", "duration=1s", "filename=" + directory);
60+
recordingClosed.await();
61+
for (Path path : Files.list(directory).toList()) {
62+
String file = path.toString();
63+
System.out.println("Found file: " + file);
64+
if (file.endsWith(".jfr") && file.contains("hotspot-")) {
65+
return;
66+
}
67+
}
68+
throw new Exception("Expected dump file on the format hotspot-...jfr");
69+
}
70+
}

0 commit comments

Comments
 (0)