RFR(s): 8210303: VM_HandshakeAllThreads fails assert with "failed: blocked and not walkable"

[Daniel D. Daugherty]

On 10/3/18 7:42 AM, Robbin Ehn wrote:
> Hi all,
>
> Notice that I missed the extended suspend case:
> http://cr.openjdk.java.net/~rehn/8210303/v2/full/

src/hotspot/share/runtime/handshake.cpp
     Don't forget to update copyright before pushing.

     L351:   if (target->is_ext_suspended()) {
     L352:     return true;
         The handshake can only be processed for a JavaThread with
         is_ext_suspended() == true when the Threads_lock is held.
         So you need:

         assert(Threads_lock->owned_by_self(), "Not holding Threads_lock.");

         at the top of the function (like you have in
         HandshakeState::vmthread_can_process_handshake()).

         You should also add a comment above L352:

           // An externally suspended thread cannot be resumed while the
           // Threads_lock is held so it is safe.

     L354:   switch(target->thread_state()) {
         Nit - please add space before '('

Thumbs up! I don't need to see a new webrev for the above tweaks.

Dan


> http://cr.openjdk.java.net/~rehn/8210303/v2/inc/
>
> Re-running sanity with t1-5 on this.
>
> Thanks, Robbin
>
> On 10/3/18 1:10 PM, Robbin Ehn wrote:
>> Hi all, please review.
>>
>> VM thread checks if it can processes a handshake for a JavaThread. That
>> check will only return a stable value if the VM thread holds the 
>> handshake
>> semaphore (or at safepoint). To avoid an unnecessary grabbing of the 
>> semaphore
>> just to release it, the VM thread do an early check to see if there 
>> is any point
>> to do the stable check. But the method 
>> SafepointSynchronize::safepoint_safe() is
>> not suppose to handle unstable checks. This can causes a false 
>> positive from an
>> assert in safepoint_safe().
>>
>> This change-set adds a local function for doing the unstable check 
>> without asserts. I do not want to expose a generic method for doing 
>> unstable safepoint safe test.
>>
>> Since asserts are not in release builds, there is no indication of a 
>> bug in JDK
>> 11. But since 11 is a LTS, this should also be considered for 
>> back-porting.
>>
>> Note, in JDK 11 only ZGC uses handshakes, previously releases have no 
>> users of handshakes.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rehn/8210303/webrev/index.html
>>
>> Bug:
>> https://bugs.openjdk.java.net/browse/JDK-8210303
>>
>> I could not reproduce it, sanity with t1-3 + handshake tests.
>>
>> Thanks, Robbin