Code review request: CR 7141259 Native stack is missing in hs_err

[Zhengyu Gu]

David,

I total agree that it is a bit of hack. The problem is that, decoder 
actually hides behind os::dll_address_to_function_name(), which makes it 
less flexible in design. Unless we are willing to mark it as 
multi-thread unsafe, and manage decoder instance and lock outside the 
scope. Native memory tracking has such flexibility, I think that the 
real issue is how to redesign dll_address_to_function_name() API.


Thanks,

-Zhengyu

On 2/6/2012 3:14 AM, David Holmes wrote:
> Hi Zhengyu,
>
> On 4/02/2012 8:12 AM, Zhengyu Gu wrote:
>> Thanks for reviewing. I think I made wrong assumption that the thread is
>> the last running thread.
>>
>> 803 // This should be the only running thread, decoder can safely run in
>> 804 // single threaded mode without acquiring lock
>> 805 Decoder::set_single_threaded(true);
>> 806
>>
>> To avoid locking in error handler, the only solution I can think of, is
>> to have its own private decoder.
>>
>> Please see revised webrev:
>> http://cr.openjdk.java.net/~zgu/7141259/webrev.01/
>
> This seems functionally ok. It is a bit strange to have a static API 
> built on the assumption of a singleton instance now having to special 
> case "is this the VM error thread" and use more than once instance, 
> but I think changing that requires revisiting the whole Decoder 
> framework.
>
> David
> -----
>
>>> Later when there are multiple users of the Decoder I don't know how
>>> you will fix this because those other users can run concurrently with
>>> report_and_die(). That said I'm unclear what it is about the Decoder
>>> that requires locking - what state does it maintain? Is there a reason
>>> not to create a Decoder only when needed?
>>>
>> Decoder is created on demand, but would prefer to be shared, since it
>> can consume noticeable amount memory on Unix platform, by loading Elf
>> files' symbol tables and string tables into memory (not impact on
>> Windows). The lock is needed to protect a linked list structure in Elf
>> decoder, or current file position.
>>
>> Thanks,
>>
>> -Zhengyu
>>> ---
>>>
>>> In decoder.cpp you can still use MutexLockerEx and as it will take a
>>> null mutex and just act as a no-op eg:
>>>
>>> MutexLockerEx locker( _single_thread ? NULL : _decoder_lock, true);
>>>
>>> Cheers,
>>> David
>>> -----
>>>
>>>>
>>>> Please review
>>>> Webrev: http://cr.openjdk.java.net/~zgu/7141259/webrev.00/
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> -Zhengyu
>>>>