RFR (S): 8149901: [Solaris] Use of -XX:+UseThreadPriorities crashes fastdebug

[serguei.spitsyn at oracle.com]

David,

It looks good to me.

Thanks,
Serguei


On 5/19/16 17:27, David Holmes wrote:
> Ping - need a Reviewer please.
>
> David
>
> On 18/05/2016 6:21 AM, David Holmes wrote:
>> On 18/05/2016 12:49 AM, Gerard Ziemski wrote:
>>> Looks good.
>>
>> Thanks.
>>
>> Can I get a Reviewer as well please.
>>
>> David
>> -----
>>
>>>
>>> thanks
>>>
>>>> On May 16, 2016, at 4:03 PM, David Holmes <david.holmes at oracle.com>
>>>> wrote:
>>>>
>>>> Hi Gerard,
>>>>
>>>> Thanks for looking at this.
>>>>
>>>> On 17/05/2016 1:19 AM, Gerard Ziemski wrote:
>>>>> hi David,
>>>>>
>>>>> The fix seems reasonable, but the following comment seems to be
>>>>> missing the end of its last sentence?
>>>>>
>>>>> +  // Most thread types will set an explicit priority before
>>>>> starting the thread,
>>>>> +  // but for those that don't we need a valid value to read back in
>>>>> thread_native_entry.
>>>>
>>>> This is the complete comment.
>>>>
>>>>> +  // Push a default initial priority into the osThread to be read
>>>>> back when the new
>>>>
>>>> This line is deleted. I forgot to refresh the webrev - now done.
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>> + osthread->set_native_priority(NormPriority);
>>>>> +
>>>>>
>>>>>
>>>>> cheers
>>>>>
>>>>>> On May 16, 2016, at 12:47 AM, David Holmes
>>>>>> <david.holmes at oracle.com> wrote:
>>>>>>
>>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8149901
>>>>>>
>>>>>> webrev: http://cr.openjdk.java.net/~dholmes/8149901/webrev/
>>>>>>
>>>>>> JDK-8038473 removed the old code pertaining to use of the T1
>>>>>> threads library and related unused flags - like
>>>>>> DefaultThreadPriority. One chunk of code it removed from
>>>>>> os::create_thread was:
>>>>>>
>>>>>> // Set the default thread priority. If using bound threads, setting
>>>>>> // lwp priority will be delayed until thread start.
>>>>>> set_native_priority(thread, DefaultThreadPriority == -1 ?
>>>>>> java_to_os_priority[NormPriority] :
>>>>>>                             DefaultThreadPriority);
>>>>>>
>>>>>> but by removing this, the logic in thread_native_entry (formerly
>>>>>> java_start) would read back an uninitialized field from the
>>>>>> OSThread instance for any thread type (eg VMThread) which did not
>>>>>> explicitly set the priority before calling os::start_thread - and
>>>>>> that would cause a range check assertion to fire.
>>>>>>
>>>>>> The simple fix, because we only deal with bound threads (and so we
>>>>>> know we will skip most of os::set_native_priority) is to just push
>>>>>> a default priority into the OSThread instance directly:
>>>>>>
>>>>>> osthread->set_native_priority(NormPriority);
>>>>>>
>>>>>> Ideally I would have fixed this in the OSThread constructor but it
>>>>>> is shared code and knows nothing about the native_priority field on
>>>>>> Solaris.
>>>>>>
>>>>>> I found a number of related issues when looking closely at this
>>>>>> code, and have filed:
>>>>>>
>>>>>> https://bugs.openjdk.java.net/browse/JDK-8157010
>>>>>>
>>>>>> as a followup cleanup for Java 10.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>
>>>