RFR: JDK-8149461: jmap kills process if non-java pid is specified in the command line
Thomas Stüfe
thomas.stuefe at gmail.com
Fri Feb 15 14:19:04 UTC 2019
Could we not just use another signal? One less... quitty? I think SIGUSR2
may still be unused, but I may be mistaken. Or, one of the real time
signals (SIGRTMIN + x).
Other than that, I still think that knowing the pid there could be platform
dependent ways of verifying the process, e.g. checking that the process has
a libjvm.so module loaded (/proc/<pid>/maps). Doing this for just one pid
should be cheap.
Cheers, Thomas
On Fri, Feb 15, 2019 at 2:46 PM Gary Adams <gary.adams at oracle.com> wrote:
> Confirmed
> -XX:-UsePerfData
>
> prevents visibility to jps, but allows direct access via pid.
>
> The new check would block access before the attach is attempted.
>
> May be best to close this bug as "will not fix".
> Requires a valid Java process pid.
>
> On 2/15/19, 8:29 AM, Bernd Eckenfels wrote:
>
> I wonder, instead of listing all VMs, would it be better to check only the
> target PID?
>
> BTW speaking of hs_perf files: is it supposed to work without
> -XX:-UserPerfData also?
>
> Gruss
> Bernd
>
> Gruss
> Bernd
> --
> http://bernd.eckenfels.net
>
> ------------------------------
> *Von:* Gary Adams <gary.adams at oracle.com> <gary.adams at oracle.com>
> *Gesendet:* Freitag, Februar 15, 2019 2:19 PM
> *An:* gary.adams at oracle.com
> *Cc:* Bernd Eckenfels; OpenJDK Serviceability
> *Betreff:* Re: RFR: JDK-8149461: jmap kills process if non-java pid is
> specified in the command line
>
> 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>
> <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>
> 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/
>> >
>> > 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.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().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/85dc20ec/attachment-0001.html>
More information about the serviceability-dev
mailing list