[RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage

Fri Sep 20 07:03:33 UTC 2019

Hello,  looks like   on Linux  there is a special check   in TestSpecialArgs.java  for 

    launcherPidString = "launcher.pid="    

that  fails after   8231171  .
Should I adjust the test ?  Or  keep the  setting in the launcher on Linux ?  

tools/launcher/TestSpecialArgs.java

        /*
         * On Linux, Launcher Tracking will print the PID.  Use this info
         * to validate what we got as the PID in the Launcher itself.
         * Linux is the only one that prints this, and trying to get it
         * here for win is awful.  So let the linux test make sure we get
         * the valid pid, and for non-linux, just make sure pid string is
         * non-zero.
         */
        if (isLinux) {
                 . . . . .
            if (launcherPid == null) {
                System.out.println(tr);
                throw new RuntimeException("Error: failed to find launcher Pid in launcher tracking info");
            }
      ...

Exception  thrown by the test  :

----begin detailed exceptions----
java.lang.reflect.InvocationTargetException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at TestHelper.run(TestHelper.java:202)
	at TestSpecialArgs.main(TestSpecialArgs.java:44)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127)
	at java.base/java.lang.Thread.run(Thread.java:830)
Caused by: java.lang.RuntimeException: Error: failed to find launcher Pid in launcher tracking info
	at TestSpecialArgs.testNativeMemoryTracking(TestSpecialArgs.java:193)
	... 12 more

Best regards, Matthias

> 
> Hi Matthias,
> 
> Thanks for cleaning this up.
> 
> On 19/09/2019 4:57 pm, Baesken, Matthias wrote:
> > Hello, as discussed below ,   I want to cleanup  some older references to
> sun.java.launcher.pid.
> > Please review the following change.
> >
> > After removal  of some code belonging  old  LinuxThreads   (JDK-8078513)   ,
> the   sun.java.launcher.pid   handling code remained  but
> >   seems to be  obsolete these days .
> >
> > Bug/webrev :
> >
> > https://bugs.openjdk.java.net/browse/JDK-8231171
> >
> > http://cr.openjdk.java.net/~mbaesken/webrevs/8231171.0/
> 
> src/hotspot/os/bsd/os_bsd.cpp
> 
> Can you delete the entire comment block here:
> 
> 1126   // Under the old bsd thread library, bsd gives each thread
> ...
> 1140   // OSThread::thread_id() method in osThread_bsd.hpp file
> 
> as it was simply copied from Linux and is nonsense on BSD.
> 
> Otherwise that all looks good to me. No need for an updated webrev.
> 
> Thanks,
> David
> -----
> 
> > Best regards, Matthias
> >
> >
> >>
> >> Hi David, thanks for the additional  information .
> >> I opened
> >>
> >> https://bugs.openjdk.java.net/browse/JDK-8231171
> >>
> >> 8231171: remove reamining sun.java.launcher.pid references
> >>
> >>    to do the additional cleanup .
> >>
> >> Best regards, Matthias
> >>
> >>
> >>> -----Original Message-----
> >>> From: David Holmes <david.holmes at oracle.com>
> >>> Sent: Mittwoch, 18. September 2019 03:16
> >>> To: Baesken, Matthias <matthias.baesken at sap.com>; 'hotspot-
> >>> dev at openjdk.java.net' <hotspot-dev at openjdk.java.net>
> >>> Subject: Re: sun.java.launcher.pid property usage
> >>>
> >>> Hi Matthias,
> >>>
> >>> On 18/09/2019 12:18 am, Baesken, Matthias wrote:
> >>>> Hello,  while looking at some  atoi usages in the codebase I started to
> >>> wonder about the  "sun.java.launcher.pid"  property.
> >>>> Currently in java_md_solinux.c  the property is set on Linux only  by
> >> default
> >>> (code is guarded #ifdef __linux__ ).
> >>>> Later it is passed  to the int  variable _sun_java_launcher_pid
> >>> (arguments.cpp) and can be retrieved by sun_java_launcher_pid() .
> >>>> However only in src/hotspot/os/bsd/os_bsd.cpp it is really used.
> >>>>
> >>>> Is the property still supported (one would need to set it from user side
> on
> >>> the command line on non-Linux because it is not set by default on
> >>> bsd/macOS) ?
> >>>> Can the coding be removed (or should it enabled for BSD/Mac like we
> do
> >>> on Linux) ?
> >>>>
> >>>> The os_bsd comment mentiones the bug 6351349 :
> >>>>
> >>>> https://bugs.java.com/bugdatabase/view_bug.do?bug_id=6351349
> >>>> JDK-6351349 : On linux with the old thread lib, jps should return the
> same
> >>> PID as $!
> >>>>
> >>>> but this looks very old.
> >>>
> >>> That was the bug that added this code as it was needed on Linux with
> >>> LinuxThreads. The code was then removed on Linux under
> >>>
> >>> https://bugs.openjdk.java.net/browse/JDK-8078513
> >>>
> >>> The review thread starts here:
> >>>
> >>> http://mail.openjdk.java.net/pipermail/hotspot-runtime-dev/2015-
> >>> May/014709.html
> >>>
> >>> I think we were focussed solely on cleaning up the hotspot Linux code
> >>> and didn't really look at the wider implication of the use of
> >>> sun.java.launcher.pid. The code in os_bsd.cpp was simply copied from
> the
> >>> Linux code without giving it any consideration - as you can tell from
> >>> the comment:
> >>>
> >>>     // With BsdThreads the JavaMain thread pid (primordial thread)
> >>>     // is different than the pid of the java launcher thread.
> >>>     // So, on Bsd, the launcher thread pid is passed to the VM
> >>>     // via the sun.java.launcher.pid property.
> >>>
> >>> where you can tell that Linux was simply replaced by Bsd, so we
> >>> reference the non-existent BsdThreads :(
> >>>
> >>> So yes this all seems to be dead code that should be removed - core-libs
> >>> folk will need to be involved of course as they own the launcher. :) It
> >>> looks like SetJavaLauncherPlatformProps() can be removed completely.
> >>>
> >>> Thanks,
> >>> David
> >