8225694: Destination option missing in FlightRecorderMXBeanImpl
Reviewed-by: egahlin
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java Mon Oct 07 10:04:01 2019 -0400
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java Mon Oct 07 16:44:12 2019 +0200
@@ -366,10 +366,16 @@
public void setDestination(WriteableUserPath userSuppliedPath) throws IOException {
synchronized (recorder) {
+ checkSetDestination(userSuppliedPath);
+ this.destination = userSuppliedPath;
+ }
+ }
+
+ public void checkSetDestination(WriteableUserPath userSuppliedPath) throws IOException {
+ synchronized (recorder) {
if (Utils.isState(getState(), RecordingState.STOPPED, RecordingState.CLOSED)) {
throw new IllegalStateException("Destination can't be set on a recording that has been stopped/closed");
}
- this.destination = userSuppliedPath;
}
}
--- a/src/jdk.jfr/share/classes/jdk/jfr/internal/management/ManagementSupport.java Mon Oct 07 10:04:01 2019 -0400
+++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/management/ManagementSupport.java Mon Oct 07 16:44:12 2019 +0200
@@ -25,6 +25,8 @@
package jdk.jfr.internal.management;
+import java.io.IOException;
+import java.nio.file.Paths;
import java.time.Duration;
import java.util.ArrayList;
import java.util.Collections;
@@ -98,4 +100,12 @@
WriteableUserPath wup = pr.getDestination();
return wup == null ? null : wup.getOriginalText();
}
+
+ public static void checkSetDestination(Recording recording, String destination) throws IOException{
+ PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
+ if(destination != null){
+ WriteableUserPath wup = new WriteableUserPath(Paths.get(destination));
+ pr.checkSetDestination(wup);
+ }
+ }
}
--- a/src/jdk.management.jfr/share/classes/jdk/management/jfr/FlightRecorderMXBeanImpl.java Mon Oct 07 10:04:01 2019 -0400
+++ b/src/jdk.management.jfr/share/classes/jdk/management/jfr/FlightRecorderMXBeanImpl.java Mon Oct 07 16:44:12 2019 +0200
@@ -28,6 +28,7 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.StringReader;
+import java.nio.file.Path;
import java.nio.file.Paths;
import java.security.AccessControlContext;
import java.security.AccessController;
@@ -105,7 +106,8 @@
private static final String OPTION_DISK = "disk";
private static final String OPTION_DUMP_ON_EXIT = "dumpOnExit";
private static final String OPTION_DURATION = "duration";
- private static final List<String> OPTIONS = Arrays.asList(new String[] { OPTION_DUMP_ON_EXIT, OPTION_DURATION, OPTION_NAME, OPTION_MAX_AGE, OPTION_MAX_SIZE, OPTION_DISK, });
+ private static final String OPTION_DESTINATION = "destination";
+ private static final List<String> OPTIONS = Arrays.asList(new String[] { OPTION_DUMP_ON_EXIT, OPTION_DURATION, OPTION_NAME, OPTION_MAX_AGE, OPTION_MAX_SIZE, OPTION_DISK, OPTION_DESTINATION, });
private final StreamManager streamHandler = new StreamManager();
private final Map<Long, Object> changes = new ConcurrentHashMap<>();
private final AtomicLong sequenceNumber = new AtomicLong();
@@ -283,6 +285,7 @@
validateOption(ops, OPTION_MAX_AGE, MBeanUtils::duration);
validateOption(ops, OPTION_MAX_SIZE, MBeanUtils::size);
validateOption(ops, OPTION_DURATION, MBeanUtils::duration);
+ validateOption(ops, OPTION_DESTINATION, x -> MBeanUtils.destination(r, x));
// All OK, now set them.atomically
setOption(ops, OPTION_DUMP_ON_EXIT, "false", MBeanUtils::booleanValue, x -> r.setDumpOnExit(x));
@@ -291,6 +294,7 @@
setOption(ops, OPTION_MAX_AGE, null, MBeanUtils::duration, x -> r.setMaxAge(x));
setOption(ops, OPTION_MAX_SIZE, "0", MBeanUtils::size, x -> r.setMaxSize(x));
setOption(ops, OPTION_DURATION, null, MBeanUtils::duration, x -> r.setDuration(x));
+ setOption(ops, OPTION_DESTINATION, null, x -> MBeanUtils.destination(r, x), x -> setOptionDestination(r, x));
}
@Override
@@ -305,6 +309,7 @@
Long maxSize = r.getMaxSize();
options.put(OPTION_MAX_SIZE, String.valueOf(maxSize == null ? "0" : maxSize.toString()));
options.put(OPTION_DURATION, ManagementSupport.formatTimespan(r.getDuration(), " "));
+ options.put(OPTION_DESTINATION, ManagementSupport.getDestinationOriginalText(r));
return options;
}
@@ -349,6 +354,20 @@
}
}
+ private static void setOptionDestination(Recording recording, String destination){
+ try {
+ Path pathDestination = null;
+ if(destination != null){
+ pathDestination = Paths.get(destination);
+ }
+ recording.setDestination(pathDestination);
+ } catch (IOException e) {
+ IllegalArgumentException iae = new IllegalArgumentException("Not a valid destination " + destination);
+ iae.addSuppressed(e);
+ throw iae;
+ }
+ }
+
private static <T, U> void validateOption(Map<String, String> options, String name, Function<String, U> validator) {
try {
String v = options.get(name);
--- a/src/jdk.management.jfr/share/classes/jdk/management/jfr/MBeanUtils.java Mon Oct 07 10:04:01 2019 -0400
+++ b/src/jdk.management.jfr/share/classes/jdk/management/jfr/MBeanUtils.java Mon Oct 07 16:44:12 2019 +0200
@@ -24,6 +24,7 @@
*/
package jdk.management.jfr;
+import java.io.IOException;
import java.lang.management.ManagementPermission;
import java.security.Permission;
import java.time.DateTimeException;
@@ -37,6 +38,7 @@
import javax.management.MalformedObjectNameException;
import javax.management.ObjectName;
+import jdk.jfr.Recording;
import jdk.jfr.internal.management.ManagementSupport;
final class MBeanUtils {
@@ -126,5 +128,16 @@
}
return size;
}
+
+ public static String destination(Recording recording, String destination) throws IllegalArgumentException{
+ try {
+ ManagementSupport.checkSetDestination(recording, destination);
+ return destination;
+ }catch(IOException e){
+ IllegalArgumentException iae = new IllegalArgumentException("Not a valid destination " + destination);
+ iae.addSuppressed(e);
+ throw iae;
+ }
+ }
}
--- a/test/jdk/jdk/jfr/jmx/TestRecordingOptions.java Mon Oct 07 10:04:01 2019 -0400
+++ b/test/jdk/jdk/jfr/jmx/TestRecordingOptions.java Mon Oct 07 16:44:12 2019 +0200
@@ -25,6 +25,7 @@
package jdk.jfr.jmx;
+import java.io.File;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.Map;
@@ -49,7 +50,7 @@
options.put("dumpOnExit", "false");
options.put("disk", "false");
options.put("duration", "1 h"); // don't want recording to stop
-
+ options.put("destination", "." + File.separator + "dump.jfr");
FlightRecorderMXBean bean = JmxHelper.getFlighteRecorderMXBean();
long recId = bean.newRecording();
Map<String, String> defaults = bean.getRecordingOptions(recId);
@@ -72,6 +73,7 @@
Asserts.assertEquals(outOptions.get("dumpOnExit"), "false", "Wrong dumpOnExit");
Asserts.assertEquals(outOptions.get("disk"), "false", "Wrong disk");
Asserts.assertEquals(outOptions.get("duration"), "1 h", "Wrong duration");
+ Asserts.assertEquals(outOptions.get("destination"), "." + File.separator + "dump.jfr", "Wrong destination");
// try empty map
bean.setRecordingOptions(recId, new HashMap<>());
@@ -116,6 +118,7 @@
nullMap.put("dumpOnExit", null);
nullMap.put("disk", null);
nullMap.put("duration", null);
+ nullMap.put("destination", null);
bean.setRecordingOptions(recId, nullMap);
Asserts.assertEquals(bean.getRecordingOptions(recId), defaults);