On 23/09/2020 7:37 pm, Robbin Ehn wrote:
On Wed, 23 Sep 2020 02:56:00 GMT, David Holmes <dhol...@openjdk.org> wrote:

Robbin Ehn has updated the pull request incrementally with one additional 
commit since the last revision:

   Update after Coleen

src/hotspot/share/runtime/handshake.hpp line 97:

95:   }
96:   bool block_for_operation() {
97:     return !_queue.is_empty() || _lock.is_locked();

I really don't understand the is_locked() check in this condition. ??
And the check for !empty is racy, so how do we avoid missing an in-progress 
addition?

A JavaThread is blocked.
A second thread have just executed a handshake operation for this JavaThread 
and are on line:
https://github.com/openjdk/jdk/blob/cd784a751a3153939b9284898f370160124ca610/src/hotspot/share/runtime/handshake.cpp#L510
And the queue is empty.

The JavaThread wakes up and changes state from blocked to blocked_trans.
It now checks if it's allowed to complete the transition to e.g. vm.

If a third thread adds to queue before the second thread leaves the loop it's 
operation can be executed.
But the JavaThread could see the queue as empty. (racey as you say)

The executor takes lock and then checks if the JavaThread is safe for 
processing.
The JavaThread becomes unsafe and then check if lock is locked.
If the lock is locked we must take slow path to avoid this.

We should also take slow path if there is something on queue to processes.
We are unsafe when we check queue and lock is not held, if we 'miss' that 
anything is on queue, it's fine.
Since any other thread cannot have seen us as safe and seen the item on queue. 
(since lock is not held)
Thus not allowed to process the operation.


Wow! That all definitely needs some detailed commentary.

Thanks,
David
-------------

PR: https://git.openjdk.java.net/jdk/pull/151

Reply via email to