RFR 8242484: Rework thread deletion during VM termination

[Patricio Chilano]

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.

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
>