RFR: 8275375: [REDO] JDK-8271949 dumppath in -XX:FlightRecorderOptions does not affect

[Erik Gahlin]

On Tue, 19 Oct 2021 03:12:32 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:

> > There are three ways setDumpPath is used:
> > 
> > 1. When JFR is initialized
> > 2. jcmd JFR.configure dumppath=
> > 3. -XX:FllightRecorderOptions:dumppath=
> > 
> > If something goes wrong (IOException/InvalidPathException) when doing 2 or 3, users should ideally be informed, so they can correct it, at command-line or jcmd. Perhaps throw DCmdException?
> 
> Agree, we can use DCmdException if IOE happens. However we need to concider 1 because some processes might be run on "/" with unprivileged user.
> 
> > Do we really need to resolve for 1? Can we handle it in native, similar to what we already do with hs_err_pid (which is not resolved in Java). For example, null could be used to indicate that the working directory should be used.
> 
> In hs_err log, the path would not resolved, it uses `os::get_current_directory()` at `VMError::prepare_log_file()` when -XX:ErrorFile is not set. For example, we can see following message on the console when we set -XX:ErrorFile=error.log, and error.log is located at current directory.
> 
> ```
> # An error report file with more information is saved as:
> # error.log
> ```
> 
> I'm not particular path resolution if we can ignore original behavior (resolved dump path will also be shown at UL). We will not have to think about SecurityException if we don't need to resolve dump path at `setDumpPath()`. In addition we can implement it more simple. So I agree with you to be honest that to pass `null`.

I'm not sure I follow completely, but how about this:

1) Remove setDumpPath(DEFAULT_DUMP_PATH) in Options::reset(). If no path has been set, os::get_current_directory() will be used when doing an emergency dump.
2) Use the SecuritySupport::canWrite check, but instead of logging, let setDumpPath throw IOException. 
3) Catch IOException in DCmdConfigure and throw DCmdExceotion, similar to what is done with repository path

Would that work?

-------------

PR: https://git.openjdk.java.net/jdk/pull/6000