Hi Chris, Serguei, Thank you for your quick review. I will push it when I receive the result from submit repo.
Yasumasa 2019年7月17日(水) 8:55 Chris Plummer <chris.plum...@oracle.com>: > On 7/16/19 4:54 PM, Chris Plummer wrote: > > On 7/16/19 4:46 PM, Chris Plummer wrote: > >> On 7/16/19 4:41 PM, Yasumasa Suenaga wrote: > >>> 2019年7月17日(水) 8:35 <serguei.spit...@oracle.com > >>> <mailto:serguei.spit...@oracle.com>>: > >>> > >>> Hi Yasumasa, > >>> > >>> Thank you for taking care about this issue! > >>> > >>> You changed the code form this: > >>> > >>> 371 } else if (cur_state == AL_NOT_INITIALIZED) { > >>> 372 // Start to initialize. > >>> 373 if (!AttachListener::is_init_trigger()) { > >>> 374 // Attach Listener could not be started. > >>> 375 // So we need to transit the state to > >>> AL_NOT_INITIALIZED. > >>> 376 AttachListener::set_state(AL_NOT_INITIALIZED); > >>> 377 } > >>> 378 continue; > >>> 379 } else if (AttachListener::check_socket_file()) { > >>> > >>> to this: > >>> > >>> 371 } else if (cur_state == AL_NOT_INITIALIZED) { > >>> 372 // Start to initialize. > >>> 373 if (AttachListener::is_init_trigger()) { > >>> 374 // Attach Listener has been initialized. > >>> 375 // Accept subsequent request. > >>> 376 continue; > >>> 377 } else { > >>> 378 // Attach Listener could not be started. > >>> 379 // So we need to transit the state to > >>> AL_NOT_INITIALIZED. > >>> 380 AttachListener::set_state(AL_NOT_INITIALIZED); > >>> 381 } > >>> 382 } else if (AttachListener::check_socket_file()) { > >>> > >>> > >>> The only difference I see is that now the continue statement is > >>> missed > >>> after the AttachListener::set_state(AL_NOT_INITIALIZED). > >>> Just want to make sure that it was your intention. > >>> > >>> > >>> Yes, we should go through when is_init_trigger() returns false. > >>> > >> I had the same question. So now my question is why is it returning > >> false? I thought this was an error condition indicating we failed to > >> initialize the attach mechanism, so I would expect this to later > >> cause the attach to fail. On the other hand, I have verified that the > >> fix seems to work. > >> > > Nevermind. I see now that AttachListener::is_init_trigger() returns > > true if the signal was being sent in order to attach, indicated by > > presence of the .attach_pid<pid> flag. So when it returns false, that > > means we want the stack trace dump instead of setting up an attach. > > > And yes, the change looks good. Thanks for the quick fix. > > Chris > > > Chris > > > >> thanks, > >> > >> Chris > >> > >>> > >>> Thanks, > >>> > >>> Yasumasa > >>> > >>> > >>> Thanks, > >>> Serguei > >>> > >>> > >>> > >>> On 7/16/19 4:15 PM, Yasumasa Suenaga wrote: > >>>> Hi all, > >>>> > >>>> Please review this change: > >>>> > >>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8227738 > >>>> webrev: > >>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8227738/webrev.00/ > >>>> > >>>> After JDK-8225690, HotSpot could not handle JVMTI dump request > >>>> (includes thread dump request). > >>>> It is caused by incorrect handling of > >>>> AttachListener::is_init_trigger(). > >>>> > >>>> I fixed it in this webrev, and it works fine on > >>>> serviceability/attach and > >>>> vmTestbase/nsk/jvmti/DataDumpRequest/datadumpreq001. > >>>> Also I pushed this to submit repo (I have not yet received the > >>>> result). > >>>> > >>>> http://hg.openjdk.java.net/jdk/submit/rev/3fb4b04ff5f2 > >>>> > >>>> > >>>> Thanks, > >>>> > >>>> Yasumasa > >>> > >> > >> > > > >
