8225694: Destination option missing in FlightRecorderMXBeanImpl
authorcito
Mon, 07 Oct 2019 16:44:12 +0200
changeset 58480 8ca46e186a63
parent 58479 35ce0ad5870a
child 58481 48a73ec3a817
8225694: Destination option missing in FlightRecorderMXBeanImpl Reviewed-by: egahlin
src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java
src/jdk.jfr/share/classes/jdk/jfr/internal/management/ManagementSupport.java
src/jdk.management.jfr/share/classes/jdk/management/jfr/FlightRecorderMXBeanImpl.java
src/jdk.management.jfr/share/classes/jdk/management/jfr/MBeanUtils.java
test/jdk/jdk/jfr/jmx/TestRecordingOptions.java
--- 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);