RFR (S) 7060111 : race condition in VMError::report_and_die()

[Coleen Phillimore]

Looks good (if you need another reviewer).
thanks,
Coleen

On 6/27/2013 10:12 AM, frederic parain wrote:
> Updated webrev:
> http://cr.openjdk.java.net/~fparain/7060111/webrev.01/
>
> Sorry for the non public bug-URLs.
>
> And I'll add your name in the Contributed-By field.
>
> Regards,
>
> Fred
>
> On 27/06/2013 15:06, Volker Simonis wrote:
>> Hi Frederic,
>>
>> great to see this fixed! (I've submitted this bug and a patch for it
>> more than two years ago:)
>>
>> I'd suggest to move the old comment for the fdSream up to the new
>> definition of  VMError::out like so:
>>
>> +// An error could happen before tty is initialized or after it has been
>> +// destroyed. Here we use a very simple unbuffered fdStream for 
>> printing.
>> +// Only out.print_raw() and out.print_raw_cr() should be used, as other
>> +// printing methods need to allocate large buffer on stack. To format a
>> +// string, use jio_snprintf() with a static buffer or use 
>> staticBufferStream.
>> +fdStream VMError::out(defaultStream::output_fd());
>> +fdStream VMError::log; // error log used by VMError::report_and_die()
>>
>> Otherwise the change looks good.
>>
>> Thanks and best regards,
>> Volker
>>
>> PS: would be great if you would not forget me in the 'Contributed-by' 
>> field :)
>>
>> PPS: please use public bug-URLs like
>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7060111 in the
>> future
>>
>>
>> On Thu, Jun 27, 2013 at 2:30 PM, frederic parain
>> <frederic.parain at oracle.com> wrote:
>>>
>>> Please review this small fix:
>>>
>>> http://cr.openjdk.java.net/~fparain/7060111/webrev.00/
>>>
>>> Bug: 7060111 : race condition in VMError::report_and_die()
>>>
>>> https://jbs.oracle.com/bugs/browse/JDK-7060111
>>>
>>> The bug is well described in its report. This fix uses
>>> class static fields initialized at library load time
>>> instead of static local variables lazily initialized.
>>>
>>> Test:
>>>
>>> The race condition is so tricky (2 threads must invoke
>>> VMError::report_and_die() almost at the same time), that I
>>> didn't try to produce a unit test for it. But the fix is
>>> so simple that it doesn't look risky.
>>>
>>> Thanks,
>>>
>>> Fred
>>>
>>> -- 
>>> Frederic Parain - Oracle
>>> Grenoble Engineering Center - France
>>> Phone: +33 4 76 18 81 17
>>> Email: Frederic.Parain at oracle.com
>