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

[David Holmes]

Thanks Serguei!

David

On 20/05/2016 3:07 PM, serguei.spitsyn at oracle.com wrote:
> 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
>>>>>>
>>>>
>