RFR 8242484: Rework thread deletion during VM termination

[Patricio Chilano]

On 4/17/20 3:24 PM, coleen.phillimore at oracle.com wrote:
> On 4/17/20 1:27 PM, Patricio Chilano wrote:
>> Hi Coleen,
>>
>> Thanks for looking at this.
>>
>> On 4/17/20 12:57 PM, coleen.phillimore at oracle.com wrote:
>>>
>>> This looks good to me also, although I'd like a reprisal of the 
>>> comment before the delete thread line that says something ilke:
>>>
>>> // We are here after VM_Exit::set_vm_exited() so we can't call
>>> // thread->smr_delete() or we will block on the Threads_lock.
>> Ok, I added back the original comment:
>>
>> diff --git a/src/hotspot/share/runtime/thread.cpp 
>> b/src/hotspot/share/runtime/thread.cpp
>> --- a/src/hotspot/share/runtime/thread.cpp
>> +++ b/src/hotspot/share/runtime/thread.cpp
>> @@ -4472,6 +4472,11 @@
>>    // exit_globals() will delete tty
>>    exit_globals();
>>
>> +  // We are here after VM_Exit::set_vm_exited() so we can't call
>> +  // thread->smr_delete() or we will block on the Threads_lock.
>> +  // Deleting the shutdown thread here is safe because another
>> +  // JavaThread cannot have an active ThreadsListHandle for
>> +  // this JavaThread.
>>    delete thread;
>>
>>> But why would it block? ... I don't see what thread has the 
>>> Threads_lock. Is this comment (that I want put back) accurate?
>> It will block because the VMThread owns the Threads_lock. The 
>> VMThread will start the terminating safepoint and exit but will never 
>> release the lock.
>
> Thanks for the explanation.  In this code, when we call 
> VMThread::wait_for_vm_thread_exit, that sets the _should_terminate 
> flag, wakes up the VMThread and the VMThread will grab the 
> Threads_lock for the final safepoint.  Therefore below at "delete 
> thread" we can't grab the Threads_lock.
Exactly.

> Comment looks good.
Thanks for the review Coleen!


Patricio
> Thanks,
> Coleen
>>
>> Patricio
>>> thanks,
>>> Coleen
>>>
>>> On 4/17/20 11:09 AM, Robbin Ehn wrote:
>>>> Hi Patricio, looks good, thanks.
>>>>
>>>> /Robbin
>>>>
>>>> On 2020-04-14 22:07, Patricio Chilano wrote:
>>>>> Hi,
>>>>>
>>>>> Please review this small follow up for 8240918 to always delete 
>>>>> the JavaThread that calls Threads::destroy_vm().
>>>>> In 8240918 I added a conditional check in Threads::destroy_vm() to 
>>>>> prevent deleting a JavaThread when there are handshakers blocked 
>>>>> in its handshake_turn_sem semaphore.  We can actually avoid this 
>>>>> issue altogether if after removing the JavaThread from the active 
>>>>> list and before triggering the terminating safepoint, we wait 
>>>>> until the JavaThread is no longer protected by a ThreadsList 
>>>>> reference.
>>>>>
>>>>> Tested in mach5 tiers 1-6.
>>>>>
>>>>> Bug:
>>>>>     https://bugs.openjdk.java.net/browse/JDK-8242484
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~pchilanomate/8242484/v1/webrev/
>>>>>
>>>>> Thanks,
>>>>> Patricio
>>>
>>
>