PING: RFR: 8209790: SA tools not providing option to connect to debug server
Yasumasa Suenaga
yasuenag at gmail.com
Mon Jul 1 22:26:21 UTC 2019
PING: Could you review it?
> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/
Yasumasa
On 2019/06/25 16:47, Yasumasa Suenaga wrote:
> Hi,
>
> This enhancement has been retargeted to 14, and the CSR has been approved.
> I uploaded a webrev for 14. Could you review it?
>
> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.03/
>
> It includes the fix to rename `--remote` to `--connect` that is
> suggested in CSR.
> It passed tests on submit repo.
>
>
> Thanks,
>
> Yasumasa
>
>
> 2019年6月17日(月) 22:11 Yasumasa Suenaga <yasuenag at gmail.com>:
>>
>> Hi David,
>>
>> On 2019/06/17 21:42, David Holmes wrote:
>>> Hi Yasumasa,
>>>
>>> On 17/06/2019 6:50 pm, Yasumasa Suenaga wrote:
>>>> Hi David,
>>>>
>>>> 8209790 is filed as a bug.
>>>
>>> I don't agree this is a "bug" - sorry. For this to be a bug there must
>>> be some specification of behaviour that the implementation is violating.
>>> Is that the case? To me this is missing functionality which makes it an
>>> enhancement.
>>
>> The feature for connecting to remote debug server has been provided JDK
>> 8 or earlier. However it was missed since JDK 9. So I think we can
>> handle it as a "bug".
>> Anyway, I added jdk13-enhancement-request label to JBS. I'm waiting for
>> the approval.
>>
>>
>>>> According to [1], I think we can push the fix to jdk/jdk13. Does it
>>>> correct?
>>>>
>>>> I'm not sure which version (13 or 14) should be set on JBS before
>>>> pushing.
>>>
>>> Just to clarify the process here, you don't want to set the "Fix
>>> Version" to 13 or 14 in JBS before pushing. That will be set based on
>>> the repo you push to. If you push to jdk/jdk it will be 14. If you push
>>> to jdk/jdk13 it will be 13. Any fix pushed to jdk/jdk13 will be
>>> "automatically" forward-ported to jdk/jdk and thus 14.
>>
>> Thanks! I got it.
>>
>>
>> Yasumasa
>>
>>
>>> David
>>> -----
>>>
>>>> (Of course I will push it after the CSR is approved.)
>>>>
>>>>
>>>> Thanks,
>>>>
>>>> Yasumasa
>>>>
>>>>
>>>> [1] http://mail.openjdk.java.net/pipermail/jdk-dev/2019-June/003051.html
>>>>
>>>>
>>>> On 2019/06/17 17:06, David Holmes wrote:
>>>>> Hi Yasumasa,
>>>>>
>>>>> On 17/06/2019 8:52 am, Yasumasa Suenaga wrote:
>>>>>> 2019年6月17日(月) 6:47 serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com> <serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com>>:
>>>>>>
>>>>>> Forgot to tell...
>>>>>> This can be pushed only after the CSR is approved.
>>>>>>
>>>>>>
>>>>>> Sure!
>>>>>> And I will push it when I get second reviewer.
>>>>>>
>>>>>> BTW should I push this change to jdk/jdk? or push to jdk/jdk13
>>>>>> manually?
>>>>>
>>>>> JDK 13 has now entered RDP1 so if you want this to go into 13 it will
>>>>> need special approval:
>>>>>
>>>>> http://openjdk.java.net/jeps/3#Late-Enhancement-Request-Process
>>>>>
>>>>> Otherwise push to jdk/jdk and it will be for JDK 14.
>>>>>
>>>>> Cheers,
>>>>> David
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> Yasumasa
>>>>>>
>>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Serguei
>>>>>>
>>>>>>
>>>>>> On 6/16/19 14:44, serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>> > Hi Yasumasa,
>>>>>> >
>>>>>> >
>>>>>> > On 6/16/19 07:22, Yasumasa Suenaga wrote:
>>>>>> >> Hi Serguei,
>>>>>> >>
>>>>>> >> >>> One minor suggestion is to use the final field NO_REMOTE
>>>>>> instead
>>>>>> >> >>> of null for initialization of the local variable "remote".
>>>>>> >>
>>>>>> >> I fixed it on new webrev. Could you check again?
>>>>>> >> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.02/
>>>>>> >
>>>>>> >
>>>>>> > It looks good.
>>>>>> > Thanks you for the update!
>>>>>> >
>>>>>> >>
>>>>>> >> >> IMHO refactoring should be worked on another issue.
>>>>>> >> >
>>>>>> >> > Agreed.
>>>>>> >>
>>>>>> >> I filed it to JBS:
>>>>>> >> https://bugs.openjdk.java.net/browse/JDK-8226204
>>>>>> >
>>>>>> > Thank you for filing the enhancement!
>>>>>> >
>>>>>> > Thanks.
>>>>>> > Serguei
>>>>>> >
>>>>>> >
>>>>>> >> Thanks,
>>>>>> >>
>>>>>> >> Yasumasa
>>>>>> >>
>>>>>> >>
>>>>>> >> On 2019/06/15 15:10, serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>> >>> Hi Yasumasa,
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> On 6/14/19 21:11, Yasumasa Suenaga wrote:
>>>>>> >>>> Hi Serguei,
>>>>>> >>>>
>>>>>> >>>> Thank you for your comment!
>>>>>> >>>>
>>>>>> >>>> On 2019/06/15 8:00, serguei.spitsyn at oracle.com
>>>>>> <mailto:serguei.spitsyn at oracle.com> wrote:
>>>>>> >>>>> Hi Yasumasa,
>>>>>> >>>>>
>>>>>> >>>>> I've added myself as a reviewer, so you can finalize it now.
>>>>>> >>>>
>>>>>> >>>> I moved CSR to Finalized, and added a comment for your
>>>>>> question.
>>>>>> >>>
>>>>>> >>> Okay, thanks!
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>>> The fix looks pretty good to me.
>>>>>> >>>>>
>>>>>> >>>>> One minor suggestion is to use the final field NO_REMOTE
>>>>>> instead
>>>>>> >>>>> of null for initialization of the local variable "remote".
>>>>>> >>>>
>>>>>> >>>> I will fix that.
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>> Also just an observation that there is some room for option
>>>>>> >>>>> processing refactoring.
>>>>>> >>>>
>>>>>> >>>> Your suggestion handles all options in one parser method.
>>>>>> >>>> I concern it might be complex for option validation.
>>>>>> >>>> (e.g. `jmap -heap` is allowed, but `jstack -heap` is not
>>>>>> allowed.)
>>>>>> >>>
>>>>>> >>> This concern is not valid as the list allowed options allowed
>>>>>> for each
>>>>>> >>> jhsdb sub-command is controlled with the longOpts argument.
>>>>>> >>>
>>>>>> >>> jmap has:
>>>>>> >>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>> "heap", "binaryheap", "dumpfile=", "histo",
>>>>>> >>> "clstats", "finalizerinfo"};
>>>>>> >>>
>>>>>> >>> but jstack has:
>>>>>> >>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>> "mixed", "locks"};
>>>>>> >>>
>>>>>> >>>> IMHO refactoring should be worked on another issue.
>>>>>> >>>
>>>>>> >>> Agreed.
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>> If you are OK in above, I will upload new webrev.
>>>>>> >>>
>>>>>> >>> Yes, I'm Okay with it.
>>>>>> >>>
>>>>>> >>>
>>>>>> >>> Thanks,
>>>>>> >>> Serguei
>>>>>> >>>
>>>>>> >>>
>>>>>> >>>> Thanks,
>>>>>> >>>>
>>>>>> >>>> Yasumasa
>>>>>> >>>>
>>>>>> >>>>
>>>>>> >>>>> All the jhsdb sub-commands do execute similar loops like
>>>>>> this:
>>>>>> >>>>> while((s = sg.next(null, longOpts)) != null) {
>>>>>> >>>>> . . .
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> It can be moved into a separate method instead.
>>>>>> >>>>> The longOpts can passed in arguments.
>>>>>> >>>>>
>>>>>> >>>>> It can be something like this:
>>>>>> >>>>>
>>>>>> >>>>> private ArrayList<String> processOptions(final String[]
>>>>>> oldArgs,
>>>>>> >>>>> final String[]
>>>>>> >>>>> longOpts,
>>>>>> >>>>> boolean
>>>>>> allowEmpty) {
>>>>>> >>>>> SAGetopt sg = new SAGetopt(oldArgs);
>>>>>> >>>>> ArrayList<String> newArgs = new ArrayList();
>>>>>> >>>>>
>>>>>> >>>>> String pid = null;
>>>>>> >>>>> String exe = null;
>>>>>> >>>>> String core = null;
>>>>>> >>>>> String s = null;
>>>>>> >>>>> String dumpfile = null;
>>>>>> >>>>> String remote = NO_REMOTE;
>>>>>> >>>>> boolean requestHeapdump = false;
>>>>>> >>>>>
>>>>>> >>>>> while((s = sg.next(null, longOpts)) != null) {
>>>>>> >>>>> if (s.equals("exe")) {
>>>>>> >>>>> exe = sg.getOptarg();
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("core")) {
>>>>>> >>>>> core = sg.getOptarg();
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("pid")) {
>>>>>> >>>>> pid = sg.getOptarg();
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("remote")) {
>>>>>> >>>>> remote = sg.getOptarg();
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("mixed")) {
>>>>>> >>>>> newArgs.add("-m");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("locks")) {
>>>>>> >>>>> newArgs.add("-l");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("heap")) {
>>>>>> >>>>> newArgs.add("-heap");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("binaryheap")) {
>>>>>> >>>>> requestHeapdump = true;
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("dumpfile")) {
>>>>>> >>>>> dumpfile = sg.getOptarg();
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("histo")) {
>>>>>> >>>>> newArgs.add("-histo");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("clstats")) {
>>>>>> >>>>> newArgs.add("-clstats");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("finalizerinfo")) {
>>>>>> >>>>> newArgs.add("-finalizerinfo");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("flags")) {
>>>>>> >>>>> newArgs.add("-flags");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("sysprops")) {
>>>>>> >>>>> newArgs.add("-sysprops");
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> if (s.equals("serverid")) {
>>>>>> >>>>> String serverid = sg.getOptarg();
>>>>>> >>>>> if (serverid != null) {
>>>>>> >>>>> newArgs.add(serverid);
>>>>>> >>>>> }
>>>>>> >>>>> continue;
>>>>>> >>>>> }
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> if (!requestHeapdump && (dumpfile != null)) {
>>>>>> >>>>> throw new IllegalArgumentException("Unexpected
>>>>>> >>>>> argument dumpfile");
>>>>>> >>>>> }
>>>>>> >>>>> if (requestHeapdump) {
>>>>>> >>>>> if (dumpfile == null) {
>>>>>> >>>>> newArgs.add("-heap:format=b");
>>>>>> >>>>> } else {
>>>>>> >>>>> newArgs.add("-heap:format=b,file=" +
>>>>>> dumpfile);
>>>>>> >>>>> }
>>>>>> >>>>> }
>>>>>> >>>>> buildAttachArgs(newArgs, pid, exe, core, remote,
>>>>>> >>>>> allowEmpty);
>>>>>> >>>>>
>>>>>> >>>>> return newArgs;
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runCLHSDB(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid="};
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, true);
>>>>>> >>>>>
>>>>>> >>>>> CLHSDB.main(newArgs.toArray(new
>>>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runHSDB(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid="};
>>>>>> >>>>> ArrayList<String> newArgs =
>>>>>> processOptions(oldArgs,
>>>>>> >>>>> longOpts, true);
>>>>>> >>>>>
>>>>>> >>>>> HSDB.main(newArgs.toArray(new
>>>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runJSTACK(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>>>> "mixed", "locks"};
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, false);
>>>>>> >>>>>
>>>>>> >>>>> JStack jstack = new JStack(false, false);
>>>>>> >>>>> jstack.runWithArgs(newArgs.toArray(new
>>>>>> >>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runJMAP(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>>>> "heap", "binaryheap", "dumpfile=", "histo",
>>>>>> >>>>> "clstats", "finalizerinfo"};
>>>>>> >>>>>
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, false);
>>>>>> >>>>>
>>>>>> >>>>> JMap.main(newArgs.toArray(new
>>>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runJINFO(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>>>> "flags", "sysprops"};
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, false);
>>>>>> >>>>>
>>>>>> >>>>> JInfo.main(newArgs.toArray(new
>>>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runJSNAP(String[] oldArgs) {
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "remote=",
>>>>>> >>>>> "all"};
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, false);
>>>>>> >>>>> JSnap.main(newArgs.toArray(new
>>>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>> private static void runDEBUGD(String[] oldArgs) {
>>>>>> >>>>> // By default SA agent classes prefer Windows
>>>>>> process
>>>>>> >>>>> debugger
>>>>>> >>>>> // to windbg debugger. SA expects special
>>>>>> properties to
>>>>>> >>>>> be set
>>>>>> >>>>> // to choose other debuggers. We will set those
>>>>>> here
>>>>>> before
>>>>>> >>>>> // attaching to SA agent.
>>>>>> >>>>>
>>>>>> System.setProperty("sun.jvm.hotspot.debugger.useWindbgDebugger",
>>>>>> >>>>> "true");
>>>>>> >>>>>
>>>>>> >>>>> String[] longOpts = {"exe=", "core=", "pid=",
>>>>>> "serverid="};
>>>>>> >>>>> ArrayList<String> newArgs = processOptions(oldArgs,
>>>>>> >>>>> longOpts, false);
>>>>>> >>>>>
>>>>>> >>>>> // delegate to the actual SA debug server.
>>>>>> >>>>> sun.jvm.hotspot.DebugServer.main(newArgs.toArray(new
>>>>>> >>>>> String[newArgs.size()]));
>>>>>> >>>>> }
>>>>>> >>>>>
>>>>>> >>>>>
>>>>>> >>>>> Please, let me know what do you think.
>>>>>> >>>>>
>>>>>> >>>>> Thanks,
>>>>>> >>>>> Serguei
>>>>>> >>>>>
>>>>>> >>>>>
>>>>>> >>>>> On 6/9/19 7:29 PM, Yasumasa Suenaga wrote:
>>>>>> >>>>>> Sorry, new webrev is here:
>>>>>> >>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.01/
>>>>>> >>>>>>
>>>>>> >>>>>> Yasumasa
>>>>>> >>>>>>
>>>>>> >>>>>> 2019年6月10日(月) 11:27 Yasumasa Suenaga<yasuenag at gmail.com
>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>> >>>>>>> PING: Could you review them?
>>>>>> >>>>>>>
>>>>>> >>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>> >>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>> >>>>>>>>>>>
>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>> It is P3 bug, but I want to fix it before JDK 13 RDP 1 if
>>>>>> possible.
>>>>>> >>>>>>>
>>>>>> >>>>>>>
>>>>>> >>>>>>> Thanks,
>>>>>> >>>>>>>
>>>>>> >>>>>>> Yasumasa
>>>>>> >>>>>>>
>>>>>> >>>>>>>
>>>>>> >>>>>>> 2019年6月5日(水) 14:06 Yasumasa Suenaga<yasuenag at gmail.com
>>>>>> <mailto:yasuenag at gmail.com>>:
>>>>>> >>>>>>>> Hi Jc,
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> Thank you for your comment!
>>>>>> >>>>>>>> I updated a webrev:
>>>>>> >>>>>>>>
>>>>>> >>>>>>>>
>>>>>> http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.01/
>>>>>> >>>>>>>>
>>>>>> >>>>>>>>> - In runTests; if DebugdUtils implemented
>>>>>> Closeable, you
>>>>>> >>>>>>>>> could just do a try-with-resources instead of the
>>>>>> finally
>>>>>> >>>>>>>>> clause...
>>>>>> >>>>>>>> I created DebugdUtils for convenience class for attach -
>>>>>> detach
>>>>>> >>>>>>>> mechanism of debug server.
>>>>>> >>>>>>>> IMHO it is prefer "detach" to "close" in this case.
>>>>>> >>>>>>>>
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> Thanks,
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> Yasumasa
>>>>>> >>>>>>>>
>>>>>> >>>>>>>>
>>>>>> >>>>>>>> 2019年6月5日(水) 11:34 Jean Christophe
>>>>>> Beyler<jcbeyler at google.com <mailto:jcbeyler at google.com>>:
>>>>>> >>>>>>>>
>>>>>> >>>>>>>>> Hi Yasumasa,
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> I'm not an official reviewer but I don't see an issue
>>>>>> with the
>>>>>> >>>>>>>>> CSR (except that this seems to be bringing a fork in the
>>>>>> tools
>>>>>> >>>>>>>>> with some handling remote and others not).
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> However, this code is really repetitive and this is
>>>>>> not the
>>>>>> >>>>>>>>> place to do a big refactor probably but we could do a
>>>>>> few
>>>>>> nits
>>>>>> >>>>>>>>> perhaps:
>>>>>> >>>>>>>>> - Instead of every tool calling commonHelp with an
>>>>>> >>>>>>>>> additional flag you could divide into commonHelp and
>>>>>> >>>>>>>>> commonHelpWithRemote for the tools and they both call
>>>>>> the
>>>>>> >>>>>>>>> current commonHelp with that boolean; so that when we
>>>>>> are
>>>>>> >>>>>>>>> looking at the tool code we know what we are
>>>>>> getting... So
>>>>>> >>>>>>>>> something like:
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> private static boolean commonHelp(String mode, boolean
>>>>>> >>>>>>>>> canConnectToRemote) {
>>>>>> >>>>>>>>> ..
>>>>>> >>>>>>>>> }
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> private static boolean commonHelp(String mode) {
>>>>>> >>>>>>>>> return commonHelp(mode, false);
>>>>>> >>>>>>>>> }
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> private static boolean commonHelpWithRemote(String
>>>>>> mode) {
>>>>>> >>>>>>>>> return commonHelp(mode, false);
>>>>>> >>>>>>>>> }
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> and that way the tools that change are just going from:
>>>>>> >>>>>>>>> - return commonHelp("jmap");
>>>>>> >>>>>>>>> + return commonHelpWithRemote("jmap");
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> - In the same vein, instead of passing null to the
>>>>>> >>>>>>>>> buildAttachArgs; you could make a variable null:
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> - buildAttachArgs(newArgs, pid, exe, core, true);
>>>>>> >>>>>>>>> + String noRemote = null;
>>>>>> >>>>>>>>> + buildAttachArgs(newArgs, pid, exe, core,
>>>>>> noRemote,
>>>>>> >>>>>>>>> true);
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>>
>>>>>> -http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/test/hotspot/jtreg/serviceability/sa/sadebugd/DebugdUtils.java.html
>>>>>>
>>>>>>
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> Nit: you have empty lines at l64 and l73
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>>
>>>>>> -http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/test/hotspot/jtreg/serviceability/sa/sadebugd/DebugdConnectTest.java.html
>>>>>>
>>>>>>
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> Nit : you have an empty line at l110
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> - In runTests; if DebugdUtils implemented
>>>>>> Closeable, you
>>>>>> >>>>>>>>> could just do a try-with-resources instead of the
>>>>>> finally
>>>>>> >>>>>>>>> clause...
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> All of these are details, I just thought I'd mention
>>>>>> them :)
>>>>>> >>>>>>>>> Jc
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> On Tue, Jun 4, 2019 at 6:44 PM Yasumasa
>>>>>> >>>>>>>>> Suenaga<yasuenag at gmail.com
>>>>>> <mailto:yasuenag at gmail.com>> wrote:
>>>>>> >>>>>>>>>> PING: Could you review them?
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>> >>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>> >>>>>>>>>>>
>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>> CSR status is provisional. So I need reviewers both
>>>>>> CSR and
>>>>>> >>>>>>>>>> webrev.
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>> Thanks,
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>> Yasumasa
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>>
>>>>>> >>>>>>>>>> 2019年5月29日(水) 22:37 Yasumasa
>>>>>> Suenaga<yasuenag at gmail.com <mailto:yasuenag at gmail.com>>:
>>>>>> >>>>>>>>>>> Hi all,
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> Please review this change:
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> JBS:https://bugs.openjdk.java.net/browse/JDK-8209790
>>>>>> >>>>>>>>>>> CSR:https://bugs.openjdk.java.net/browse/JDK-8224979
>>>>>> >>>>>>>>>>>
>>>>>> webrev:http://cr.openjdk.java.net/~ysuenaga/JDK-8209790/webrev.00/
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> In JDK 8 or earlier, some tools (jstack, jmap,
>>>>>> jinfo) can
>>>>>> >>>>>>>>>>> connect to
>>>>>> >>>>>>>>>>> debug server (jsadebugd). However it has not done
>>>>>> so since
>>>>>> >>>>>>>>>>> JDK 9 because
>>>>>> >>>>>>>>>>> jhsdb cannot accept the attach request to debug
>>>>>> server.
>>>>>> >>>>>>>>>>> So I want to introduce new option `--remote` to
>>>>>> connect to
>>>>>> >>>>>>>>>>> debug server.
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> I created CSR for this issue. So please review it
>>>>>> together.
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> Thanks,
>>>>>> >>>>>>>>>>>
>>>>>> >>>>>>>>>>> Yasumasa
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> --
>>>>>> >>>>>>>>>
>>>>>> >>>>>>>>> Thanks,
>>>>>> >>>>>>>>> Jc
>>>>>> >>>>>
>>>>>> >>>
>>>>>> >
>>>>>>
More information about the serviceability-dev
mailing list