RFR: JDK-8149461: jmap kills process if non-java pid is specified in the command line
Gary Adams
gary.adams at oracle.com
Fri Feb 15 13:20:22 UTC 2019
On a linux system with 1 Java process and 500 non-Java processes,
/tmp is not tmpfs mounted, 20 runs average 255.5 ms stddev 9.32.
JDK-8176828 is the issue that enabled perfmemory visibility once the vm
is in live mode.
For containers that are configured without a shared view of /tmp, it may
be necessary
to include a override of the pid check.
On 2/15/19, 7:29 AM, Gary Adams wrote:
> I'll get some measurements on some local systems so we have a
> specific idea about the overhead of the pid check.
> I imagine in most production environments the /tmp tmpfs
> is memory only set of checks.
>
> One of the "not all vms are recognized" was fixed recently.
> When starting a libjdwp session with server=y and suspend=y,
> the vm was not recognized until a debugger was attached.
>
> I'm not opposed to adding a force option to skip the valid pid
> check. It might be better effort fixing the hung vm case if we
> have a concrete failure to investigate.
>
> On 2/15/19, 6:47 AM, Bernd Eckenfels wrote:
>> Hello,
>>
>> I see possible issues here, not sure if they still exist but I wanted
>> to mention them:
>>
>> the list-vm function might be slow on a loaded system (as it is a
>> complex function). It’s not the best Situation if your diagnostic
>> attempts are slow down in such a situation.
>>
>> Also in the past not all VMs might be recognized by the list
>> function, a more targeted attach could still succeed. Is that
>> addressed since the container-PID changes? In both cases a force
>> option would help.
>>
>> Gruss
>> Bernd
>>
>> Gruss
>> Bernd
>> --
>> http://bernd.eckenfels.net
>> ------------------------------------------------------------------------
>> *Von:* serviceability-dev
>> <serviceability-dev-bounces at openjdk.java.net> im Auftrag von
>> gary.adams at oracle.com
>> *Gesendet:* Freitag, Februar 15, 2019 12:07 PM
>> *An:* Thomas Stüfe; David Holmes; Chris Plummer
>> *Cc:* OpenJDK Serviceability
>> *Betreff:* Re: RFR: JDK-8149461: jmap kills process if non-java pid
>> is specified in the command line
>> Let me clarify a few things about the proposed fix.
>>
>> The VirtualMachine.list() mechanism is based on
>> Java processes that are up and running.
>> During VM startup when agent libraries are loaded,
>> the fact is recorded in the filesystem that a Java process
>> is eligible for an attach request.
>>
>> This is a much smaller list than scanning for all the
>> running processes and works across all supported
>> platforms. It also doesn't rely on fragile command line
>> parsing to determine a Java launcher is used.
>>
>> I believe most of the reported hang situations
>> are not for the first level information for pid and
>> command line requests. I believe the hangs are due
>> to the second level requests that actually attach
>> to the process and issue a command to the running
>> Java process.
>>
>> The overhead for the pid check should be relatively small.
>> In a standalone command for a one time check at startup
>> with 10's of Java processes. I know the documentation
>> states the user is responsible for verifying with jps
>> that a Java process pid or vmid is provided. But the cost
>> of human error can be a terminated process.
>>
>> Selection by name also has some drawbacks when multiple
>> copies of a command are running at the same time.
>>
>> Running "jcmd 0 ..." is the documented way to run a command on
>> all running Java processes.
>>
>> May I count you as a reviewer on the current changeset?
>>
>> On 2/15/19 3:54 AM, Thomas Stüfe wrote:
>>>
>>>
>>> On Fri, Feb 15, 2019 at 3:26 AM David Holmes
>>> <david.holmes at oracle.com <mailto:david.holmes at oracle.com>> wrote:
>>>
>>> Gary,
>>>
>>> What is the overhead of doing the validation? How do we list
>>> VMs? Do we
>>> need to examine every running process to get the list of VMs?
>>> Wouldn't
>>> it be better to check the given process is a VM rather than
>>> checking all
>>> potential VM processes?
>>>
>>>
>>> I think this is a valid point. Listing VMs is normally quick (just
>>> use jcmd without any parms) but I have seen this fail or hang rarely
>>> in situations where I then still was able to talk to my VM via pid.
>>> I never investigated this but I would not like to be unable to
>>> connect to my VM just because some rogue VM mailfunctions.
>>>
>>> This would be an argument for the alternative I offered in my first
>>> answer - just use the proc file system or a similar mechanism to
>>> check only the pid you plan on sending sigquit to...
>>>
>>> I think there is an onus of responsibility on the user to provide a
>>> correct pid here, rather than trying to make this foolproof.
>>>
>>>
>>> Oh but it can happen quite easily. For example, by pulling up an
>>> older jcmd from your shell history and forgetting to modify the pid.
>>> Thank god for the command name argument option on jcmd, which I now
>>> use mostly.
>>>
>>> We also had a customer which tried to find all VMs on his machine by
>>> attempting to attach with jcmd to every process, killing them left
>>> and right :)
>>>
>>> ... Thomas
>>>
>>> Thanks,
>>> David
>>>
>>> On 15/02/2019 5:12 am, Gary Adams wrote:
>>> > The following commands present a similar kill process behavior:
>>> > jcmd
>>> > jinfo
>>> > jmap
>>> > jstack
>>> >
>>> > The following commands do not attach.
>>> > jstat sun.jvmstat.monitor.MonitorException "not found"
>>> > jps no pid arguments
>>> >
>>> > This update moves the checkJavaPid method into the
>>> > common/ProcessArgumentsMatcher.java
>>> > and applies the check before the pid is used for an attach
>>> operation.
>>> >
>>> > Revised webrev:
>>> http://cr.openjdk.java.net/~gadams/8149461/webrev.01/
>>> <http://cr.openjdk.java.net/%7Egadams/8149461/webrev.01/>
>>> >
>>> > On 2/14/19, 12:07 PM, Chris Plummer wrote:
>>> >> Hi Gary,
>>> >>
>>> >> What about the other tools that attach to a user specified
>>> process. Do
>>> >> they also have this issue?
>>> >>
>>> >> thanks,
>>> >>
>>> >> Chris
>>> >>
>>> >> On 2/14/19 8:35 AM, Gary Adams wrote:
>>> >>> There is an old reported problem that using jmap on a
>>> process that is
>>> >>> not running
>>> >>> Java could cause the process to terminate. This is due to
>>> the SIGQUIT
>>> >>> used
>>> >>> when attaching to the process.
>>> >>>
>>> >>> It is a fairly simple operation to validate that the pid
>>> matches one
>>> >>> of the known
>>> >>> running Java processes using VirtualMachine.list().
>>> >>>
>>> >>> Issue: https://bugs.openjdk.java.net/browse/JDK-8149461
>>> >>>
>>> >>> Proposed fix:
>>> >>>
>>> >>> diff --git
>>> a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>> >>> b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>> >>> --- a/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>> >>> +++ b/src/jdk.jcmd/share/classes/sun/tools/jmap/JMap.java
>>> >>> @@ -1,5 +1,5 @@
>>> >>> /*
>>> >>> - * Copyright (c) 2005, 2018, Oracle and/or its affiliates. All
>>> >>> rights reserved.
>>> >>> + * Copyright (c) 2005, 2019, Oracle and/or its affiliates. All
>>> >>> rights reserved.
>>> >>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE
>>> HEADER.
>>> >>> *
>>> >>> * This code is free software; you can redistribute it
>>> and/or modify it
>>> >>> @@ -30,6 +30,7 @@
>>> >>> import java.io.InputStream;
>>> >>> import java.io <http://java.io>.UnsupportedEncodingException;
>>> >>> import java.util.Collection;
>>> >>> +import java.util.List;
>>> >>>
>>> >>> import com.sun.tools.attach.VirtualMachine;
>>> >>> import com.sun.tools.attach.VirtualMachineDescriptor;
>>> >>> @@ -125,6 +126,10 @@
>>> >>> private static void executeCommandForPid(String pid,
>>> String
>>> >>> command, Object ... args)
>>> >>> throws AttachNotSupportedException, IOException,
>>> >>> UnsupportedEncodingException {
>>> >>> + // Before attaching, confirm that pid is a known
>>> Java process
>>> >>> + if (!checkJavaPid(pid)) {
>>> >>> + throw new AttachNotSupportedException();
>>> >>> + }
>>> >>> VirtualMachine vm = VirtualMachine.attach(pid);
>>> >>>
>>> >>> // Cast to HotSpotVirtualMachine as this is an
>>> >>> @@ -196,6 +201,19 @@
>>> >>> executeCommandForPid(pid, "dumpheap", filename,
>>> liveopt);
>>> >>> }
>>> >>>
>>> >>> + // Check that pid matches a known running Java process
>>> >>> + static boolean checkJavaPid(String pid) {
>>> >>> + List<VirtualMachineDescriptor> l =
>>> VirtualMachine.list();
>>> >>> + boolean found = false;
>>> >>> + for (VirtualMachineDescriptor vmd: l) {
>>> >>> + if (vmd.id <http://vmd.id>().equals(pid)) {
>>> >>> + found = true;
>>> >>> + break;
>>> >>> + }
>>> >>> + }
>>> >>> + return found;
>>> >>> + }
>>> >>> +
>>> >>> private static void checkForUnsupportedOptions(String[]
>>> args) {
>>> >>> // Check arguments for -F, -m, and non-numeric value
>>> >>> // and warn the user that SA is not supported anymore
>>> >>
>>> >>
>>> >
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190215/7d190c80/attachment-0001.html>
More information about the serviceability-dev
mailing list