PING: RFR: 8209790: SA tools not providing option to connect to debug server

Yasumasa Suenaga yasuenag at gmail.com
Tue Jun 25 07:47:46 UTC 2019


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