RFR: 8027429: Add diagnostic command VM.info to get hs_err print-out
Coleen Phillimore
coleen.phillimore at oracle.com
Thu Oct 29 12:02:24 UTC 2015
On 10/29/15 7:58 AM, David Holmes wrote:
> On 29/10/2015 9:26 PM, Coleen Phillimore wrote:
>>
>> So I actually disagree. I don't think there should be an additional
>> separate mechanism to get the same information that we get with hs_err
>> reporting. I've wanted to have a feature like this for a long time.
>>
>> I pre-reviewed this change and I thought it looked good in general. I
>> didn't see the thread iteration problem that Fred refers to below, but I
>> think the individual problems can be fixed.
>>
>> The last thing I want is this code to be copied somewhere else.
>
> Factored as needed not copied. VMError is not an "info" reporting
> mechanism.
If you look at the things that are reported in each "STEP", there's a
small amount of code and the order is important.
I'd like the vm info to use the same order and report what it can do
safely. Refactoring 5 lines of code into functions doesn't make sense.
Coleen
>
> David
>
>> Coleen
>>
>> On 10/28/15 8:48 PM, David Holmes wrote:
>>> I agree with Fred, this kind of info reporting should not be
>>> piggy-backed onto VMError handling for the reasons stated (and the
>>> error handling logic is complicated enough as it is!). For things like
>>> thread lists there are already safe management functions that
>>> can/should be used.
>>>
>>> Thanks,
>>> David H.
>>>
>>> On 29/10/2015 3:29 AM, Frederic Parain wrote:
>>>> Hi David,
>>>>
>>>> I haven't review all the code yet, but I'm concerned with the
>>>> fact that the diagnostic command is calling VMError::report().
>>>> This method has been implemented to be executed in the particular
>>>> context of fatal errors, and its usage while the VM is running
>>>> normally seems dangerous.
>>>>
>>>> For instance, VMError::report() consciously avoids grabbing locks
>>>> because of the risk of deadlock during the error reporting.
>>>> The consequence is that some data structures are browsed in
>>>> an unsafe way. One example: VMError::report() calls
>>>> Threads::print_on_error() which iterates over the thread
>>>> list *without owning the Threads_lock*.
>>>>
>>>> The implementation of the diagnostic command seems also to
>>>> exclude a lot of reporting from the initial VMError::report()
>>>> method. Have you considered implementing a new MT-safe reporting
>>>> method rather than trying to modify the special VMError::report()
>>>> methods? (Note that some code factorization between VMError::report()
>>>> and this new method should be possible).
>>>>
>>>> Thanks,
>>>>
>>>> Fred
>>>>
>>>> On 28/10/2015 17:18, david buck wrote:
>>>>> Hi!
>>>>>
>>>>> Please review my change for this small enhancement.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8027429
>>>>> webrev: http://cr.openjdk.java.net/~dbuck/8027429_jdk9_01/
>>>>>
>>>>> Cheers,
>>>>> -Buck
>>>>
>>
More information about the hotspot-runtime-dev
mailing list