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

[frederic parain]

Thank you Coleen, and yes, I needed a second review
from a HSX Reviewer.

Fred

On 28/06/2013 22:47, Coleen Phillimore wrote:
>
> 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
>>
>

-- 
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: Frederic.Parain at oracle.com