On Fri, Feb 15, 2019 at 3:26 AM David Holmes
<david.hol...@oracle.com <mailto:david.hol...@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
<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
>>
>>
>
