RFR: 8029630: Print thread id hex representation in error messages.
Thomas Stüfe
thomas.stuefe at gmail.com
Sun Apr 5 08:44:22 UTC 2015
Hi Max,
sorry, I missed that current_thread_id() is defined as intx, which means
that your change in vmError.cpp is not correct. %x will only print 32bit
values.
For hex values, I guess your best bet would be either PTR_FORMAT or
something like SIZE_FORMAT_HEX, depending on whether you want to print the
value with leading zeros or not.
Regards, Thomas
On Fri, Apr 3, 2015 at 7:34 PM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:
> Hi Max,
>
> I guess both would be fine: leave it at %x or add a new format specifier
> macro. We also could deal with a better abstraction for thread ids in a
> later change.
>
> ..Thomas
>
> On Fri, Apr 3, 2015 at 5:55 PM, Max Ockner <max.ockner at oracle.com> wrote:
>
>> Thomas,
>> Thanks for pointing out the flaw in thread.cpp. I will fix that.
>> I originally looked for an abstraction but couldn't find one. If you
>> think it would be better to create a new format specifier, then I will do
>> that as well.
>> Max
>>
>> On 4/3/2015 5:42 AM, Thomas Stüfe wrote:
>>
>> Hi Max,
>>
>> Disclaimer: not a Reviewer.
>>
>> In thread.cpp, the code prints the value of Thread* pointer, not a
>> thread id. So the code was correct before.
>>
>> That said, I find this pretty confusing, especially because it prints
>> "tid=<this>". Maybe one should change this or throw it away.
>>
>> --
>>
>> As for vmError.cpp: this seems to be correct.
>>
>> However, I would like to see this abstracted somehow, here is why:
>>
>> The OpenJDK simply assumes thread id to be 32bit unsigned. This is not
>> really portable and may bite us in the face (I am not sure if it already
>> did, was not thread id 64bit on AIX? at least for kernel thread ids?).
>>
>> If you use pthread_t to implement thread id, like many ports do, thread
>> id may be anything - 64bit number, array, struct. POSIX leaves that open.
>>
>> So, instead of a raw "%x", I propose as quick fix a format specifier
>> for thread ids, e.g. "THREAD_ID_FORMAT". That way we could at least easily
>> find all places where we print thread ids, if we later decide to change the
>> type for thread ids.
>>
>> Regards, Thomas
>>
>>
>> On Thu, Apr 2, 2015 at 9:38 PM, Max Ockner <max.ockner at oracle.com> wrote:
>>
>>> Hello,
>>> Please review this change:
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8029630
>>> Webrev: http://cr.openjdk.java.net/~mockner/8029630.2/
>>> Summary: Existing error messages for vm crashes print the decimal
>>> representation of the current thread id. The message format has been
>>> changed to print the hex value of the thread id.
>>>
>>> Tested with jtreg runtime to make sure nothing was broken. I tested the
>>> new functionality separately. I crashed the vm and examined the debugger
>>> output to make sure the hexadecimal thread id printed in the error message
>>> matched one of the threads listed by the debugger.
>>>
>>> Thanks,
>>> Max
>>>
>>
>>
>>
>
More information about the hotspot-dev
mailing list