<trimming>
On 23/09/2020 8:11 pm, Robbin Ehn wrote:
On Wed, 23 Sep 2020 03:04:39 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.cpp line 63:
61: };
62:
63: class AsyncHandshakeOperation : public HandshakeOperation {
This doesn't quite make sense. If you have an AsyncHandshakeOperation as a
distinct subclass then it should not be
possible for is_async() on a HandshakeOperation to return true - but it can
because it can be passed an
AsyncHandshakeClosure when constructed. If you want async and non-async
operations to be distinct types then you will
need to restrict how the base class is constructed, and provide a protected
constructor that just takes an
AsyncHandShakeClosure.
This implementation code not part of the interface.
I find it hard to tell which classes form which.
By casting the AsyncHandShakeClosure to a HandshakeClosure before instantiating
the HandshakeOperation you can still
get is_async() to return true. And there are a loads of other user error which
can be done like stack allocating
AsyncHandshakeOperation. Protecting against all those kinds of errors requires
a lot of more code.
Can we at least declare a protected constructor for HandshakeOperation
that takes the AsyncHandshakeClosure, so that an accidental creation of
"new HandShakeperation(anAsyncClosure)" will be prevented?
Thanks,
David
-------------
PR: https://git.openjdk.java.net/jdk/pull/151
