8221730: jcmd process name matching broken
Jean Christophe Beyler
jcbeyler at google.com
Fri Apr 5 16:40:13 UTC 2019
Hi Daniil,
Looks good to me too :)
Jc
On Thu, Apr 4, 2019 at 9:05 PM David Holmes <david.holmes at oracle.com> wrote:
> Great!
>
> Thanks,
> David
>
> On 5/04/2019 2:00 pm, Daniil Titov wrote:
> > Hi David,
> >
> > I figured out what went wrong. I used webrev.ksh with the list of
> files to diff. Using "webrev.ksh -m -N" solved the problem. The new
> webrev is uploaded as webrev.04.
> >
> > Webrev: http://cr.openjdk.java.net/~dtitov/8221730/webrev.04
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
> >
> > Thanks,
> > Daniil
> >
> > On 4/4/19, 7:15 PM, "David Holmes" <david.holmes at oracle.com> wrote:
> >
> > Hi Daniil,
> >
> > On 5/04/2019 12:01 pm, Daniil Titov wrote:
> > > Hi David,
> > >
> > > I didn't use "hg rename". Now I recreated the patch using "hg
> rename" for moving
> test/hotspot/jtreg/serviceability/dcmd/framework/TestJavaProcess.java to
> test/hotspot/jtreg/serviceability/dcmd/framework/process/TestJavaProcess.java.
> I also used the latest version of webrev.ksh (25.17) to re-create a webrev
> (I uploaded it as webrev.03). However, I still don't see that renaming is
> somehow reflected in the webrev. Not sure what might be wrong here. The hg
> version is 4.5.3, the OS is Ubuntu 18.04.2 LTS.
> >
> > Strange. Here's an example of where the rename shows up in the
> webrev:
> >
> > http://cr.openjdk.java.net/~dholmes/8213233/webrev/
> >
> > Anyway as long as it was done with hg rename that is fine.
> >
> > Thanks,
> > David
> > ----
> >
> > >
> > > Webrev: http://cr.openjdk.java.net/~dtitov/8221730/webrev.03/
> > > Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
> > >
> > > Thanks!
> > >
> > > Best regards,
> > > Daniil
> > >
> > > On 4/4/19, 5:45 PM, "David Holmes" <david.holmes at oracle.com>
> wrote:
> > >
> > > Hi Daniil,
> > >
> > > On 5/04/2019 8:17 am, Daniil Titov wrote:
> > > > Hi David and JC,
> > > >
> > > > Thank you for reviewing this change. Please review a new
> version of
> > > > the fix that adds additional tests as David suggested. The
> tests are
> > > > added to
> > > >
> test/hotspot/jtreg/serviceability/dcmd/framework/VMVersionTest.java and
> > > > test the cases when Java process is started with -jar or
> -m (--module)
> > > > options.
> > >
> > > Looks good - thanks for doing that.
> > >
> > > > Since unnamed packages are not allowed in the modules, file
> > > >
> test/hotspot/jtreg/serviceability/dcmd/framework/TestJavaProcess.java
> > > > was moved to
> > > >
> test/hotspot/jtreg/serviceability/dcmd/framework/process/TestJavaProcess.java.
> > >
> > > Did you use "hg rename" for that? The webrev doesn't show
> that you did.
> > >
> > > Thanks,
> > > David
> > >
> > > >
> > > > Webrev:
> http://cr.openjdk.java.net/~dtitov/8221730/webrev.02/
> > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8221730
> > > >
> > > > Thanks!
> > > > --Daniil
> > > >
> > > >
> > > > On 4/3/19, 5:23 PM, "David Holmes" <
> david.holmes at oracle.com> wrote:
> > > >
> > > > Hi Daniil,
> > > >
> > > > This seems reasonable, but can we add a suitable
> regression test please
> > > > to verify behaviour before JDK-8205654 and with this
> fix.
> > > >
> > > > Thanks,
> > > > David
> > > >
> > > > On 4/04/2019 5:02 am, Daniil Titov wrote:
> > > > > Please review the change that makes jcmd process
> name matching on Linux platform consistent with pre-existing behavior.
> > > > >
> > > > > On other platforms (and on Linux platform before
> changes [3]) the jcmd uses system property "sun.rt.javaCommand" (see
> sun.jvmstat.monitor. MonitoredVmUtil.mainClass(MonitoredVm, boolean) that
> is called from sun.tools.common. ProcessArgumentMatcher at line 96) and
> treats the part before the first space as a main class when matching the
> process name. However, if the application is started with -jar option this
> part contains the path to the jar file. If -m or --module option is used
> this part contains the module name and the main class (if the main class
> was specified in the command line) in the format <modulename>/<mainclass>.
> After changes [3] , on Linux platform the proc filesystem is used to find a
> Java process, however, it always matches the process name against the main
> class regardless what options were used to launch the application. This
> created discrepancies between old and new behavior on Linux platform as
> well as between behavior on Linux and other platforms. Th!
> > > > > e fix changes sun.tool.ProcessHelper (that was
> introduced in [3]) to correct these discrepancies.
> > > > >
> > > > >
> > > > > Reference:
> > > > > [1] Webrev:
> http://cr.openjdk.java.net/~dtitov/8221730/webrev.01
> > > > > [2] Bug:
> https://bugs.openjdk.java.net/browse/JDK-8221730
> > > > > [3]
> https://bugs.openjdk.java.net/browse/JDK-8205654
> > > > >
> > > > >
> > > > > Thanks!
> > > > > --Daniil
> > > > >
> > > > >
> > > >
> > > >
> > > >
> > >
> > >
> > >
> >
> >
> >
>
--
Thanks,
Jc
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190405/48ec9b5c/attachment.html>
More information about the serviceability-dev
mailing list