2019年7月17日(水) 9:02 Yasumasa Suenaga <yasue...@gmail.com>: > Hi Chris, Serguei, > > Thank you for your quick review. > I will push it when I receive the result from submit repo. >
I received the result from submit repo, this change works fine. mach5-one-ysuenaga-JDK-8227738-20190716-2315-3843771 I will push it tonight (in Japan) 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 >> >>> >> >> >> >> >> > >> >>
