JDK-8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Dec 15 15:47:07 UTC 2017
On 12/15/17 10:18 AM, Gary Adams wrote:
> On 12/15/17, 9:14 AM, Daniel D. Daugherty wrote:
>> On 12/15/17 9:05 AM, Gary Adams wrote:
>>> Looking into some minor bugs in the attach area for the next release.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8188856
>>>
>>> Seems like a minor change to fix this error message that mentions
>>> the java_pid socket file,
>>> but then outputs the attach_pid file when the socket file is not
>>> created .
>>>
>>> What is the general preference for this type fix?
>>> - remove the reference to the "socket file" in the error message
>>> - construct the filename for the socket that was not found
>>
>> Just an opinion...
>>
>> Having the specific pathname for the <socket_file> that cannot be
>> found is a better diagnostic. Perhaps it's possible to refactor the
>> code that generates the <socket_file> path so that both sides can
>> share that function to generate the same name...
>>
>> It might not be possible to do this sharing, e.g., one side might
>> be in Java and the other side might be in C... In that case, having
>> the same algorithm implemented in both places with comments to link
>> the two would be my next choice...
>>
>> Of course, you have to balance this level of effort against other
>> things on your plate...
>>
>> Dan
>>
> So one minimal solution would be to save the socket file name, so
> it's available when the error message is created.
This seems like a reasonable solution. Please wait for Serviceability
team folks to chime in though... :-)
Dan
>
> diff --git
> a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java
> b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java
> --- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java
> +++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java
> @@ -47,8 +47,9 @@
> // Any changes to this needs to be synchronized with HotSpot.
> private static final String tmpdir = "/tmp";
>
> - // The patch to the socket file created by the target VM
> + // The path to the socket file created by the target VM
> String path;
> + String socket_name;
>
> /**
> * Attaches to the target VM
> @@ -98,7 +99,7 @@
> throw new AttachNotSupportedException(
> String.format("Unable to open socket file %s:
> " +
> "target process %d doesn't respond within
> %dms " +
> - "or HotSpot VM not loaded", f.getPath(),
> pid, time_spend));
> + "or HotSpot VM not loaded", socket_name,
> pid, time_spend));
> }
> } finally {
> f.delete();
> @@ -267,10 +268,11 @@
> // Return the socket file for the given process.
> private String findSocketFile(int pid) {
> File f = new File(tmpdir, ".java_pid" + pid);
> + socket_name = f.getPath();
> if (!f.exists()) {
> return null;
> }
> - return f.getPath();
> + return socket_name;
> }
>
> // On Solaris/Linux/Aix a simple handshake is used to start the
> attach mechanism
> diff --git
> a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
> b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
> ---
> a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
> +++
> b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
> @@ -48,8 +48,9 @@
> // Any changes to this needs to be synchronized with HotSpot.
> private static final String tmpdir = "/tmp";
>
> - // The patch to the socket file created by the target VM
> + // The path to the socket file created by the target VM
> String path;
> + String socket_name;
>
> /**
> * Attaches to the target VM
> @@ -102,7 +103,8 @@
> throw new AttachNotSupportedException(
> String.format("Unable to open socket file %s:
> " +
> "target process %d doesn't respond within
> %dms " +
> - "or HotSpot VM not loaded", f.getPath(),
> pid, time_spend));
> + "or HotSpot VM not loaded", socket_name, pid,
> + time_spend));
> }
> } finally {
> f.delete();
> @@ -275,10 +277,11 @@
> // procfs regardless of namespaces.
> String root = "/proc/" + pid + "/root/" + tmpdir;
> File f = new File(root, ".java_pid" + ns_pid);
> + socket_name = f.getPath();
> if (!f.exists()) {
> return null;
> }
> - return f.getPath();
> + return socket_name;
> }
>
> // On Solaris/Linux a simple handshake is used to start the
> attach mechanism
> diff --git
> a/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
> b/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
> ---
> a/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
> +++
> b/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
> @@ -46,8 +46,9 @@
> // Any changes to this needs to be synchronized with HotSpot.
> private static final String tmpdir;
>
> - // The patch to the socket file created by the target VM
> + // The path to the socket file created by the target VM
> String path;
> + String socket_name;
>
> /**
> * Attaches to the target VM
> @@ -96,8 +97,9 @@
> if (path == null) {
> throw new AttachNotSupportedException(
> String.format("Unable to open socket file %s:
> " +
> - "target process %d doesn't respond within
> %dms " +
> - "or HotSpot VM not loaded", f.getPath(),
> pid, time_spend));
> + "target process %d doesn't respond within %dms " +
> + "or HotSpot VM not loaded", socket_name,
> + pid, time_spend));
> }
> } finally {
> f.delete();
> @@ -269,7 +271,8 @@
> private String findSocketFile(int pid) {
> String fn = ".java_pid" + pid;
> File f = new File(tmpdir, fn);
> - return f.exists() ? f.getPath() : null;
> + socket_name = f.getPath();
> + return f.exists() ? socket_name : null;
> }
>
More information about the serviceability-dev
mailing list