RFR (S): 8139801: Error message from validation check has wrong order on Windows

[sangheon.kim]

Hi Chris,

Thank you for reviewing this.
My answers are below.

On 10/20/2015 04:35 PM, Chris Plummer wrote:
> Hi Sangheon,
>
> Thanks for doing this. A few comments/suggestions:
>
> I don't think the #ifdef __WIN32__ should be needed. Although we are 
> doing the fflush mainly to address issues we only currently see on 
> win32, there's no reason why the fflush can't be done on all 
> platforms. I think that will help keep the code cleaner.
 From cleaner code point of view, I totally agree with you.
Let me hear others opinion on this.

>
> In jni.cpp, is there a reason not to flush unconditionally (outside 
> the if/else)?
Hmm.
I thought we will not have strings to be flushed from the create_vm() 
success case.
But I think I have to move outside of 'if statement'.

>
> Please make sure your commit comment references all 3 CRs this fix is 
> addressing:
>
> 8139374 TlabSize.java argument test fails
> 8078328 AppCDS jtreg test result missing expected error message in stderr
> 8139801 Error message from validation check has wrong order on Windows
Okay.

Thanks,
Sangheon


>
> I'm not a "R"eviewer.
>
> thanks,
>
> Chris
>
> On 10/20/15 3:03 PM, sangheon.kim wrote:
>> Hi all,
>>
>> Initially I only tested validation error messages but later I noticed 
>> that other error messages also have same problem on Windows. (thanks 
>> to Chris Plummer for the discussion)
>>
>> When we fail to create VM, we end with 1) returning to the caller or 
>> 2) call vm_abort().
>> For the 1) case, all error messages have wrong order on Windows.
>>
>> eg) an error message on Windows. (this is NG)
>> $ java -XX:VMOptionsFile=foo -XX:VMOptionsFile=bar
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>> Only one VM Options file is supported on the command line  <- this 
>> should be printed first.
>>
>> We can see easily this wrong order on Windows but the problem is 
>> there's no guarantee that 'stdout/stderr' will be flushed on that 
>> situation.
>>
>> As a solution I added fflush(stdout) and fflush(stderr) for above two 
>> cases.
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8139801
>> Webrev: http://cr.openjdk.java.net/%7Esangheki/8139801/webrev.01/
>> Testing: JPRT
>>
>> Thanks,
>> Sangheon
>>
>>
>> On 10/19/2015 01:00 PM, sangheon.kim wrote:
>>> Hi all again,
>>>
>>> Let me cancel this patch as this kind of problem seems general on 
>>> Windows.
>>>
>>> Thanks,
>>> Sangheon
>>>
>>>
>>> On 10/19/2015 08:56 AM, sangheon.kim wrote:
>>>> Hi all,
>>>>
>>>> Can I get some reviews for this change of adding 'fflush()' on 
>>>> validation message print?
>>>>
>>>> The order of 'validation error message' and 'vm exit message' is 
>>>> wrong on Windows.
>>>> - Windows is buffering the validation error message which is sent 
>>>> to 'stderr'.
>>>> - 'stderr' from 'jvm.dll' is flushed later than 'stderr' from the 
>>>> caller of 'jvm.dll'.
>>>>
>>>> eg) Expected message order (all other platforms except Windows)
>>>> $ java -XX:MinTLABSize=1 -version
>>>> 1) MinTLABSize (1) must be greater than or equal to reserved area 
>>>> in TLAB (16)
>>>> 2) Error: Could not create the Java Virtual Machine. \n Error: A 
>>>> fatal exception has occurred. Program will exit.
>>>>
>>>> On Windows:
>>>> $ java -XX:MinTLABSize=1 -version
>>>> 2) Error: Could not create the Java Virtual Machine. \n Error: A 
>>>> fatal exception has occurred. Program will exit.
>>>> 1) MinTLABSize (1) must be greater than or equal to reserved area 
>>>> in TLAB (16)
>>>>
>>>> As a solution, I added 'fflush()' on print function of validation 
>>>> check (CommandLineError::print).
>>>> And any other functions that are printing to 'stderr' would have 
>>>> similar fix for Windows.
>>>>
>>>> CR: https://bugs.openjdk.java.net/browse/JDK-8139801
>>>> Webrev: http://cr.openjdk.java.net/~sangheki/8139801/webrev.00/
>>>> Testing: JPRT
>>>>
>>>> Thanks,
>>>> Sangheon
>>>>
>>>
>>
>