JDK-8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix

[Gary Adams]

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.

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;
      }