RFR: 8261391: ZGC crash - SEGV in RevokeOneBias::do_thread [v3]

[Robbin Ehn]

On Thu, 18 Feb 2021 02:10:56 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Tiny spelling error
>
> src/hotspot/share/runtime/handshake.cpp line 84:
> 
>> 82:  public:
>> 83:   AsyncHandshakeOperation(AsyncHandshakeClosure* cl, JavaThread* target, jlong start_ns)
>> 84:     : HandshakeOperation(cl, target, NULL), _start_time_ns(start_ns) {}
> 
> Not at all clear why requester is always NULL here. There is obviously a requester thread even in the async case. If that thread created a Handle in the op then don't we need to call start_processing on it?

The asynchronous handshake may never be executed, thus having an infinity life-time. (thread never leaves native/blocked)
Referring to a the requesting thread means we must keep it alive forever also.
If you refer to stack-based resources from a thread, means that thread should not touch it's stack before you are done.
If the requester can't use it stack until you are done, you essentially have a synchronous handshake.

The only way we have to keep the requesting thread alive is to put in a ThreadsList, which means we would keep all threads alive potentially forever (until end of VM-life). But still a Handle would be unsafe because it's stack-based.

Therefore it's hard-coded to NULL. (are we suppose to use nullptr now ?)

> src/hotspot/share/runtime/handshake.cpp line 328:
> 
>> 326: 
>> 327: void Handshake::execute(HandshakeClosure* hs_cl) {
>> 328:   HandshakeOperation cto(hs_cl, NULL, Thread::current());
> 
> It is unfortunate that we need to manifest the current thread here (and elsewhere).

Yes, I did that since I did not want to change the API.
I was thinking of:
Handshake::execute(HandshakeClosure* hs_cl, Thread* requester = NULL) 
And then go over calls site and see if we have the current thread in a variable.
But I felt that would be better off in another change, if wanted.

-------------

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