RFR (S): 8245509: Crash handler itself crashes when reporting Unsafe.putInt(0) crash
coleen.phillimore at oracle.com
coleen.phillimore at oracle.com
Mon Jun 1 23:17:52 UTC 2020
On 6/1/20 7:13 PM, David Holmes wrote:
> On 2/06/2020 8:45 am, coleen.phillimore at oracle.com wrote:
>> On 6/1/20 6:36 PM, David Holmes wrote:
>>> On 2/06/2020 7:21 am, coleen.phillimore at oracle.com wrote:
>>>> On 6/1/20 3:57 PM, gerard ziemski wrote:
>>>>> hi Coleen,
>>>>>
>>>>> On 6/1/20 1:33 PM, coleen.phillimore at oracle.com wrote:
>>>>>> In rev2, in the second part, can you move the ResourceMark just
>>>>>> out of the loop? Otherwise, looks good and I don't need to see
>>>>>> another webrev.
>>>>>
>>>>> I thought the ResourceMark should go with the operation it's meant
>>>>> for, to make the code more self documenting.
>>>
>>> Yes but you don't have to put it right before the op especially in
>>> loops. I only had it in the loop because I was not applying this
>>> change:
>>>
>>> - if (_verbose && _context && Universe::is_fully_initialized()) {
>>> + if (_verbose && _context && _thread &&
>>> Universe::is_fully_initialized()) {
>>>
>>> which I do not agree with. We are now excluding printing this
>>> information if we don't have an attached thread. It was okay to
>>> exclude "printing register info" because the next section printed
>>> similar info for when we do not have a _thread. It isn't clear to me
>>> that is the case here.
>>>
>>> I would prefer not to print less information if we don't have to
>>> exclude it.
>>
>> If there's no current thread (which is 100% unlikely in this case) it
>> doesn't make any sense to print it's stack.
>
> Well we already try to do just that - if we crash while attaching the
> current thread for example. But in that case a bunch of error
> reporting steps break anyway, including this one:
>
> Stack slot to memory mapping:
> stack at sp + 0 slots:
> [error occurred during error reporting (inspecting top of stack), id
> 0xe0000000, Internal Error
> (/jdk-dev/open/src/hotspot/share/runtime/thread.hpp:855)]
>
> So I guess we are not losing anything. :(
I had a look at this code this morning, and am not sure most of it would
work with a null _thread and _verbose == true. Thanks for testing it out.
Coleen
>
> David
>
>> Coleen
>>
>>>
>>> David
>>> -----
>>>
>>>>>
>>>>> Why does it need to go outside pf the loop? To minimize number of
>>>>> allocations?
>>>>
>>>> It sets an allocation marker in the Arena to allocate from in the
>>>> current thread. It's a pretty cheap operation, and it's for error
>>>> handling, so it can go inside the loop. Your change is fine.
>>>>
>>>> Coleen
>>>>
>>>>>
>>>>>
>>>>> cheers
>>>>
>>
More information about the hotspot-runtime-dev
mailing list