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

[Coleen Phillimore]

Sangheon, this change looks good.
Coleen

On 10/23/15 6:57 PM, sangheon.kim wrote:
> Hi all,
>
> I'm waiting some reviews from (R)eviewer.
> And here's a new webrev which includes Chris' comments.
> - Move fflush() outside of 'if' statement at jni.cpp.
> - Remove #ifdef _WIN32
>
> webrev:
> http://cr.openjdk.java.net/~sangheki/8139801/webrev.02/
>
> Thanks,
> Sangheon
>
>
> On 10/20/2015 05:17 PM, sangheon.kim wrote:
>> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>