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

[David Holmes]

Correction ...

On 18/02/2021 12:22 pm, David Holmes wrote:
> 
> More generally I am concerned by the fact that we have a potential bug if a Handle created by one thread is accessed by another, without first calling start_processing() on the creator of the Handle. This would seem to be error prone to me unless it is actually hidden away inside the Handle implementation - though how to do that efficiently is unclear. I'm very concerned that it is too easy to write code (eg. handing something off to the  service thread or notification thread) that will be buggy.

I was confusing Handles with Resources in terms of lifecycle - not 
realising Handles are stack-local to a Thread and can only be shared 
with another thread very, very carefully.

David

> Perhaps @fisk can comment on the general problem.
> 
> Thanks,
> David
> 
> 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?
> 
> 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).
> 
> -------------
> 
> Changes requested by dholmes (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/2571
>