RFR 8240918: [REDO] Allow direct handshakes without VMThread intervention

[coleen.phillimore at oracle.com]

On 4/2/20 3:45 AM, David Holmes wrote:
> Hi Patricio,
>
> On 2/04/2020 3:45 am, Patricio Chilano wrote:
>> Hi all,
>>
>> Please review this redo for 8230594. Since the reason behind the 
>> Windows timeouts turned out to be 8240902, this patch is almost 
>> identical to the reverse of the backout modulo the rebase.
>
> Glad to see this come back! Overall changes still look good - naturally.
>
>> There is only one small fix for an issue not identified in the 
>> original patch which I explained in the comments. To avoid this 
>> possible issue of deleting a JavaThread that called 
>> Thread::destroy_vm() while there are handshakers blocked in the 
>> handshake_turn_sem semaphore, I added a check to verify the 
>> JavaThread in question is not protected by some ThreadsList reference 
>> before attempting the delete it. In case it is protected, we need to 
>> at least clear the value for _thread_key to avoid possible infinite 
>> loops at thread exit (this can happen in Solaris).
>
> Interesting problem. At some point during the shutdown sequence the 
> thread needs to be able to execute "process any pending handshake and 
> mark this thread as no longer handshake-capable". But it is unclear 
> how to achieve that at this time so I'm okay with deferring this to a 
> future RFE. Even though the VM has "terminated" this may be a custom 
> launcher, or a hosted VM, and the main process is not necessarily 
> going away, so it would be preferable to be able to delete the thread 
> and free all its resources.

Something that was surprising to me was that no other threads are 
deleted or resources freed, so why free this one?

It was yet another amazing feat of debugging that Patricio found the 
side effect of the destructor that matters in this case.

Coleen
>
> One nit: in the ThreadSMR code this looks really odd:
>
>  static bool is_a_protected_JavaThread_with_lock(JavaThread *thread, 
> bool skiplock = false);
>
> A "with lock" function that might not lock? I suggest just making 
> is_a_protected_JavaThread public and calling it directly.
>
> Thanks,
> David
> -----
>
>> The only other change is that I removed the check for local polls in 
>> Handshake::execute_direct() since after the backout, thread-local 
>> handshakes was implemented for arm32 which was the only remaining 
>> platform.
>>
>> I tested it several times in mach5 tiers 1-6 and once in t7 and saw 
>> no failures.
>>
>> Bug:
>>     https://bugs.openjdk.java.net/browse/JDK-8240918
>>
>> Webrev:
>>     http://cr.openjdk.java.net/~pchilanomate/8240918/v1/webrev/
>>
>>
>> Thanks,
>> Patricio