RFR 8242484: Rework thread deletion during VM termination

[coleen.phillimore at oracle.com]

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.

Comment looks good.
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
>>
>