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

[Harold David Seigel]

Thanks Serguei!

Harold


On 12/20/2018 8:40 PM, serguei.spitsyn at oracle.com wrote:
> Hi Harold,
>
> It looks good to me.
>
> Thanks,
> Serguei
>
> On 12/20/18 1:30 PM, Harold David Seigel wrote:
>> 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
>>>>
>>
>