[RFR] 8231171: remove remaining sun.java.launcher.pid references - was RE: sun.java.launcher.pid property usage
David Holmes
david.holmes at oracle.com
Fri Sep 20 08:11:09 UTC 2019
Hi Matthias,
On 20/09/2019 5:03 pm, Baesken, Matthias wrote:
>
> 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 ?
IMHO adjust the test please.
Thanks,
David
-----
>
> 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
>>>
More information about the hotspot-dev
mailing list