8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix
authorgadams
Tue, 02 Jan 2018 07:50:17 -0500
changeset 48485 258a4dab74a7
parent 48484 9ca19ebea22d
child 48486 bda5211e7876
8188856: Incorrect file path in an exception message when .java_pid is not accessible on Unix Reviewed-by: cjplummer, sspitsyn
src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java
src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java
src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java
src/jdk.attach/solaris/classes/sun/tools/attach/VirtualMachineImpl.java
--- a/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Dec 05 10:43:23 2017 +0000
+++ b/src/jdk.attach/aix/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Jan 02 07:50:17 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2008, 2017, Oracle and/or its affiliates. All rights reserved.
  * Copyright (c) 2015 SAP SE. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
@@ -46,9 +46,7 @@
     // will not be able to find all Hotspot processes.
     // 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
-    String path;
+    String socket_path;
 
     /**
      * Attaches to the target VM
@@ -69,8 +67,9 @@
         // Find the socket file. If not found then we attempt to start the
         // attach mechanism in the target VM by sending it a QUIT signal.
         // Then we attempt to find the socket file again.
-        path = findSocketFile(pid);
-        if (path == null) {
+        File socket_file = new File(tmpdir, ".java_pid" + pid);
+        socket_path = socket_file.getPath();
+        if (!socket_file.exists()) {
             File f = createAttachFile(pid);
             try {
                 sendQuitTo(pid);
@@ -86,19 +85,19 @@
                     try {
                         Thread.sleep(delay);
                     } catch (InterruptedException x) { }
-                    path = findSocketFile(pid);
 
                     time_spend += delay;
-                    if (time_spend > timeout/2 && path == null) {
+                    if (time_spend > timeout/2 && !socket_file.exists()) {
                         // Send QUIT again to give target VM the last chance to react
                         sendQuitTo(pid);
                     }
-                } while (time_spend <= timeout && path == null);
-                if (path == null) {
+                } while (time_spend <= timeout && !socket_file.exists());
+                if (!socket_file.exists()) {
                     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_path, pid,
+                                      time_spend));
                 }
             } finally {
                 f.delete();
@@ -107,14 +106,14 @@
 
         // Check that the file owner/permission to avoid attaching to
         // bogus process
-        checkPermissions(path);
+        checkPermissions(socket_path);
 
         // Check that we can connect to the process
         // - this ensures we throw the permission denied error now rather than
         // later when we attempt to enqueue a command.
         int s = socket();
         try {
-            connect(s, path);
+            connect(s, socket_path);
         } finally {
             close(s);
         }
@@ -125,8 +124,8 @@
      */
     public void detach() throws IOException {
         synchronized (this) {
-            if (this.path != null) {
-                this.path = null;
+            if (socket_path != null) {
+                socket_path = null;
             }
         }
     }
@@ -144,12 +143,10 @@
         assert args.length <= 3;            // includes null
 
         // did we detach?
-        String p;
         synchronized (this) {
-            if (this.path == null) {
+            if (socket_path == null) {
                 throw new IOException("Detached from target VM");
             }
-            p = this.path;
         }
 
         // create UNIX socket
@@ -157,7 +154,7 @@
 
         // connect to target VM
         try {
-            connect(s, p);
+            connect(s, socket_path);
         } catch (IOException x) {
             close(s);
             throw x;
@@ -264,15 +261,6 @@
         }
     }
 
-    // Return the socket file for the given process.
-    private String findSocketFile(int pid) {
-        File f = new File(tmpdir, ".java_pid" + pid);
-        if (!f.exists()) {
-            return null;
-        }
-        return f.getPath();
-    }
-
     // On Solaris/Linux/Aix a simple handshake is used to start the attach mechanism
     // if not already started. The client creates a .attach_pid<pid> file in the
     // target VM's working directory (or temp directory), and the SIGQUIT handler
--- a/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Dec 05 10:43:23 2017 +0000
+++ b/src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Jan 02 07:50:17 2018 -0500
@@ -47,10 +47,7 @@
     // will not be able to find all Hotspot processes.
     // 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
-    String path;
-
+    String socket_path;
     /**
      * Attaches to the target VM
      */
@@ -73,8 +70,9 @@
         // Find the socket file. If not found then we attempt to start the
         // attach mechanism in the target VM by sending it a QUIT signal.
         // Then we attempt to find the socket file again.
-        path = findSocketFile(pid, ns_pid);
-        if (path == null) {
+        File socket_file = findSocketFile(pid, ns_pid);
+        socket_path = socket_file.getPath();
+        if (!socket_file.exists()) {
             File f = createAttachFile(pid, ns_pid);
             try {
                 sendQuitTo(pid);
@@ -90,19 +88,19 @@
                     try {
                         Thread.sleep(delay);
                     } catch (InterruptedException x) { }
-                    path = findSocketFile(pid, ns_pid);
 
                     time_spend += delay;
-                    if (time_spend > timeout/2 && path == null) {
+                    if (time_spend > timeout/2 && !socket_file.exists()) {
                         // Send QUIT again to give target VM the last chance to react
                         sendQuitTo(pid);
                     }
-                } while (time_spend <= timeout && path == null);
-                if (path == null) {
+                } while (time_spend <= timeout && !socket_file.exists());
+                if (!socket_file.exists()) {
                     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_path, pid,
+                                      time_spend));
                 }
             } finally {
                 f.delete();
@@ -111,14 +109,14 @@
 
         // Check that the file owner/permission to avoid attaching to
         // bogus process
-        checkPermissions(path);
+        checkPermissions(socket_path);
 
         // Check that we can connect to the process
         // - this ensures we throw the permission denied error now rather than
         // later when we attempt to enqueue a command.
         int s = socket();
         try {
-            connect(s, path);
+            connect(s, socket_path);
         } finally {
             close(s);
         }
@@ -129,8 +127,8 @@
      */
     public void detach() throws IOException {
         synchronized (this) {
-            if (this.path != null) {
-                this.path = null;
+            if (socket_path != null) {
+                socket_path = null;
             }
         }
     }
@@ -148,12 +146,10 @@
         assert args.length <= 3;                // includes null
 
         // did we detach?
-        String p;
         synchronized (this) {
-            if (this.path == null) {
+            if (socket_path == null) {
                 throw new IOException("Detached from target VM");
             }
-            p = this.path;
         }
 
         // create UNIX socket
@@ -161,7 +157,7 @@
 
         // connect to target VM
         try {
-            connect(s, p);
+            connect(s, socket_path);
         } catch (IOException x) {
             close(s);
             throw x;
@@ -257,8 +253,9 @@
             if ((off < 0) || (off > bs.length) || (len < 0) ||
                 ((off + len) > bs.length) || ((off + len) < 0)) {
                 throw new IndexOutOfBoundsException();
-            } else if (len == 0)
+            } else if (len == 0) {
                 return 0;
+            }
 
             return VirtualMachineImpl.read(s, bs, off, len);
         }
@@ -269,16 +266,12 @@
     }
 
     // Return the socket file for the given process.
-    private String findSocketFile(int pid, int ns_pid) {
+    private File findSocketFile(int pid, int ns_pid) {
         // A process may not exist in the same mount namespace as the caller.
         // Instead, attach relative to the target root filesystem as exposed by
         // procfs regardless of namespaces.
         String root = "/proc/" + pid + "/root/" + tmpdir;
-        File f = new File(root, ".java_pid" + ns_pid);
-        if (!f.exists()) {
-            return null;
-        }
-        return f.getPath();
+        return new File(root, ".java_pid" + ns_pid);
     }
 
     // On Solaris/Linux a simple handshake is used to start the attach mechanism
--- a/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Dec 05 10:43:23 2017 +0000
+++ b/src/jdk.attach/macosx/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Jan 02 07:50:17 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2017, 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
@@ -45,9 +45,7 @@
     // the latter can be changed by the user.
     // 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
-    String path;
+    String socket_path;
 
     /**
      * Attaches to the target VM
@@ -68,8 +66,9 @@
         // Find the socket file. If not found then we attempt to start the
         // attach mechanism in the target VM by sending it a QUIT signal.
         // Then we attempt to find the socket file again.
-        path = findSocketFile(pid);
-        if (path == null) {
+        File socket_file = new File(tmpdir, ".java_pid" + pid);
+        socket_path = socket_file.getPath();
+        if (!socket_file.exists()) {
             File f = createAttachFile(pid);
             try {
                 sendQuitTo(pid);
@@ -85,19 +84,19 @@
                     try {
                         Thread.sleep(delay);
                     } catch (InterruptedException x) { }
-                    path = findSocketFile(pid);
 
                     time_spend += delay;
-                    if (time_spend > timeout/2 && path == null) {
+                    if (time_spend > timeout/2 && !socket_file.exists()) {
                         // Send QUIT again to give target VM the last chance to react
                         sendQuitTo(pid);
                     }
-                } while (time_spend <= timeout && path == null);
-                if (path == null) {
+                } while (time_spend <= timeout && !socket_file.exists());
+                if (!socket_file.exists()) {
                     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_path,
+                                      pid, time_spend));
                 }
             } finally {
                 f.delete();
@@ -106,14 +105,14 @@
 
         // Check that the file owner/permission to avoid attaching to
         // bogus process
-        checkPermissions(path);
+        checkPermissions(socket_path);
 
         // Check that we can connect to the process
         // - this ensures we throw the permission denied error now rather than
         // later when we attempt to enqueue a command.
         int s = socket();
         try {
-            connect(s, path);
+            connect(s, socket_path);
         } finally {
             close(s);
         }
@@ -124,8 +123,8 @@
      */
     public void detach() throws IOException {
         synchronized (this) {
-            if (this.path != null) {
-                this.path = null;
+            if (socket_path != null) {
+                socket_path = null;
             }
         }
     }
@@ -143,12 +142,10 @@
         assert args.length <= 3;                // includes null
 
         // did we detach?
-        String p;
         synchronized (this) {
-            if (this.path == null) {
+            if (socket_path == null) {
                 throw new IOException("Detached from target VM");
             }
-            p = this.path;
         }
 
         // create UNIX socket
@@ -156,7 +153,7 @@
 
         // connect to target VM
         try {
-            connect(s, p);
+            connect(s, socket_path);
         } catch (IOException x) {
             close(s);
             throw x;
@@ -264,14 +261,6 @@
         }
     }
 
-    // Return the socket file for the given process.
-    // Checks temp directory for .java_pid<pid>.
-    private String findSocketFile(int pid) {
-        String fn = ".java_pid" + pid;
-        File f = new File(tmpdir, fn);
-        return f.exists() ? f.getPath() : null;
-    }
-
     /*
      * Write/sends the given to the target VM. String is transmitted in
      * UTF-8 encoding.
@@ -282,7 +271,7 @@
             try {
                 b = s.getBytes("UTF-8");
             } catch (java.io.UnsupportedEncodingException x) {
-                throw new InternalError();
+                throw new InternalError(x);
             }
             VirtualMachineImpl.write(fd, b, 0, b.length);
         }
--- a/src/jdk.attach/solaris/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Dec 05 10:43:23 2017 +0000
+++ b/src/jdk.attach/solaris/classes/sun/tools/attach/VirtualMachineImpl.java	Tue Jan 02 07:50:17 2018 -0500
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2017, 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
@@ -47,6 +47,7 @@
 
     // door descriptor;
     private int fd = -1;
+    String socket_path;
 
     /**
      * Attaches to the target VM
@@ -60,7 +61,7 @@
         try {
             pid = Integer.parseInt(vmid);
         } catch (NumberFormatException x) {
-            throw new AttachNotSupportedException("invalid process identifier");
+            throw new AttachNotSupportedException("Invalid process identifier");
         }
 
         // Opens the door file to the target VM. If the file is not
@@ -100,7 +101,7 @@
                     throw new AttachNotSupportedException(
                         String.format("Unable to open door %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_path, pid, time_spend));
                 }
             } finally {
                 f.delete();
@@ -210,13 +211,13 @@
 
     // The door is attached to .java_pid<pid> in the temporary directory.
     private int openDoor(int pid) throws IOException {
-        String path = tmpdir + "/.java_pid" + pid;;
-        fd = open(path);
+        socket_path = tmpdir + "/.java_pid" + pid;
+        fd = open(socket_path);
 
         // Check that the file owner/permission to avoid attaching to
         // bogus process
         try {
-            checkPermissions(path);
+            checkPermissions(socket_path);
         } catch (IOException ioe) {
             close(fd);
             throw ioe;