RFR(s): 8222327: java_lang_Thread _thread_status_offset, remove pre 1.5 code paths

David Holmes david.holmes at oracle.com
Tue Apr 16 10:42:36 UTC 2019


On 16/04/2019 6:50 pm, Robbin Ehn wrote:
> Hi David,
> 
> On 4/16/19 10:27 AM, David Holmes wrote:
>> Hi Robbin,
>>
>> On 16/04/2019 6:17 pm, Robbin Ehn wrote:
>>> Hi Claes and David,
>>>
>>> Here is v2:
>>> http://cr.openjdk.java.net/~rehn/8222327/v2/
>>
>> This is missing any check that the offsets are always initialized 
>> before use. Can you guarantee none of these will be used in such a 
>> case? If so asserts at least would be good.
> 
> The other ones don't have assert.
> 
> 1698 bool java_lang_Thread::is_daemon(oop java_thread) {
> 1699   return java_thread->bool_field(_daemon_offset) != 0;
> 1700 }
> 1701
> 1702
> 1703 void java_lang_Thread::set_daemon(oop java_thread) {
> 1704   java_thread->bool_field_put(_daemon_offset, true);
> 1705 }
> 
>>
>>    oop java_lang_Thread::park_blocker(oop java_thread) {
>> !   assert(JDK_Version::current().supports_thread_park_blocker(),
>> !          "Must support parkBlocker field");
>>
>> This assert can be removed as can 
>> JDK_Version::supports_thread_park_blocker() and possibly other stuff, 
>> as they also represent pre-1.5 code paths.
> 
> Yes, I know, but this is part of the JDK_Version API, which means I get 
> a load of collateral changes. And I did not want to do cleanup 
> JDK_Version in this patch.

I thought you were on a crusade to get rid of pre JDK 1.5 code ;-) Okay 
we can leave this to another cleanup.

>  From other mail:
>  >If the offset is not initialized you will now crash (most likely) 
> instead of >getting a zero etc. This would only be a potential issue 
> during VM >initialization, and possibly only on initialization failures 
> which makes this >very hard to test.
> 
> But the other ones don't have assert so that is the case today.

So they could potentially both be wrong.

As it is this code is initialized via init_globals() before any Java 
Thread object can be created, so there is no issue with access.

Thanks,
David

> Thanks, Robbin
> 
> 
>>
>> Thanks,
>> David
>> -----
>>
>>> Inc:
>>> http://cr.openjdk.java.net/~rehn/8222327/v2/inc/
>>>
>>> Passed t1-2.
>>>
>>> Thanks, Robbin
>>>
>>> On 4/15/19 11:04 AM, Robbin Ehn wrote:
>>>> Hi all, please review.
>>>>
>>>> Removing some dead code.
>>>>
>>>> Code:
>>>> http://cr.openjdk.java.net/~rehn/8222327/webrev/
>>>> Issue:
>>>> https://bugs.openjdk.java.net/browse/JDK-8222327
>>>>
>>>> Passes t1-5.
>>>>
>>>> Thanks, Robbin


More information about the hotspot-runtime-dev mailing list