RFR 8215398: -Xlog option usage => Invalid decorator '\temp\app_cds.log'.

[Harold David Seigel]

Hi David,

Thanks for looking at this!

Please review this updated webrev.  The fix is the same but the webrev 
contains a new test instead of modifying an existing test:

    http://cr.openjdk.java.net/~hseigel/bug_8215398.2/webrev/

The logging implementation does not handle single quotes as expected.  
For example, if the TestQuotedLogOutputs.java test is changed to use 
single quotes instead of double quotes, it will fail, even on Linux, 
because the single quotes are included as part of the file name.  They 
are not stripped off.  That is why "java 
-Xlog:safepoint=trace:'d:\safepointtrace.txt' -version" fails.

Perhaps a new bug is needed to change logging's handling of single 
quotes?  It's a bit surprising that this issue hasn't already been reported.

Thanks, Harold

On 12/20/2018 2:07 AM, David Holmes wrote:
> Hi Harold,
>
> On 19/12/2018 11:22 pm, Harold David Seigel wrote:
>> Hi,
>>
>> Please review this small change to fix JDK-8215398.  The fix works by 
>> not treating ':' as a delimiter in a -Xlog... option string if it is 
>> following by a '\' and preceded by either a single character or the 
>> text 'file='.  The fix is for Windows only.
>>
>> Open Webrev: 
>> http://cr.openjdk.java.net/~hseigel/bug_8215398/webrev/index.html
>
> I think I can follow the fix.
>
> But I'm a bit concerned about the test. AFAICS the test thought it was 
> already testing this case - albeit with the path quoted:
>
>   42         // Ensure log files can be specified with full path.
>   43         // On windows, this means that the file name will contain
>   44         // a colon ('C:\log.txt' for example), which is used to
>   45         // separate -Xlog: options (-Xlog:tags:filename:decorators).
>   46         // Try to log to a file in our current directory, using 
> its absolute path.
>   47         String baseName = "test file.log";
>   48         Path filePath = Paths.get(baseName).toAbsolutePath();
>   49         String fileName = filePath.toString();
>   50         File file = filePath.toFile();
> ...
>   65         String[] validOutputs = new String[] {
>   66             quote + fileName + quote,
>   67             "file=" + quote + fileName + quote,
>   68             quote + fileName + quote + ":",
>   69             quote + fileName + quote + "::"
>   70         };
>
> But even quoted if I specify the drive designator in the path, it 
> fails! Here's a local attempt at this:
>
> D:\ade> apps\Java\jdk-11\fastdebug\bin\java 
> -Xlog:safepoint=trace:'d:\safepointtrace.txt' -version
> [0.014s][error][logging] Invalid decorator '\safepointtrace.txt''.
> Invalid -Xlog option '-Xlog:safepoint=trace:'d:\safepointtrace.txt'', 
> see error log for details.
> Error: Could not create the Java Virtual Machine.
> Error: A fatal exception has occurred. Program will exit.
>
> So AFAICS the test can't possibly have been testing what it thought it 
> was testing! ???
>
> I'm also not sure about just adding some unquoted variants to the 
> existing TestQuotedLogOutputs without renaming the test and updating 
> the @summary
>
> Thanks,
> David
>
>> JBS Bug: https://bugs.openjdk.java.net/browse/JDK-8215398
>>
>> The fix was regression tested by running Mach5 tiers 1 and 2 tests 
>> and builds on Linux-x64, Windows, and Mac OS X, running tiers 3-5 
>> tests on Linux-x64, running JCK-12 Lang and VM tests on Linux-x64, 
>> and by hand on Windows.
>>
>> Thanks, Harol
>>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20181220/8657c4fa/attachment.html>