On Sat, 18 Nov 2023 02:29:26 GMT, Alex Menkov <amen...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: add jdk_internal_vm_Continuation::done(cont) check to
>> JvmtiEnvBase::is_vthread_alive
>
> src/hotspot/share/prims/jvmtiEnvBase.cpp line 1989:
>
>> 1987: } else {
>> 1988: Handshake::execute(hs_cl, tlh, target_jt); // delegate to
>> Handshake implementation
>> 1989: }
>
> Every implementation of JvmtiUnitedHandshakeClosure has to check if the
> target thread is virtual and call do_vthread manually.
> I'd suggest to handle this by proxy class, something like
> Suggestion:
>
> class Adapter : public HandshakeClosure {
> JvmtiUnitedHandshakeClosure* _hs_cl;
> Handle _target_h;
> public:
> Adapter(JvmtiUnitedHandshakeClosure* hs_cl, Handle target_h)
> : HandshakeClosure(hs_cl->name()), _hs_cl(hs_cl),
> _target_h(target_h) {}
> virtual void do_thread(Thread* thread) {
> if (java_lang_VirtualThread::is_instance(_target_h())) { // virtual
> thread
> _hs_cl->do_vthread(_target_h);
> } else {
> _hs_cl->do_thread(target);
> }
> }
> } adapter(hs_cl, target_h);
>
> if (self) { // target thread is current
> adapter.do_thread(target_jt); // execute handshake closure callback on
> current thread directly
> } else {
> Handshake::execute(&adapter, tlh, target_jt); // delegate to Handshake
> implementation
> }
Thank you for the suggestion! Agreed, this should help to get rid of this
duplication/ugliness.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16460#discussion_r1398218934
