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