-
Notifications
You must be signed in to change notification settings - Fork 6.1k
8319589: Attach from root to a user java process not supported in Mac #25824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
9fd3781
f35e7e6
779a1b3
de855d3
9ca4aaf
4c96cca
75dd6fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,9 +28,13 @@ | |
import com.sun.tools.attach.AttachNotSupportedException; | ||
import com.sun.tools.attach.spi.AttachProvider; | ||
|
||
import sun.jvmstat.PlatformSupport; | ||
|
||
import java.io.InputStream; | ||
import java.io.IOException; | ||
import java.io.File; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
|
||
import static java.nio.charset.StandardCharsets.UTF_8; | ||
|
||
|
@@ -39,14 +43,17 @@ | |
*/ | ||
@SuppressWarnings("restricted") | ||
public class VirtualMachineImpl extends HotSpotVirtualMachine { | ||
// "tmpdir" is used as a global well-known location for the files | ||
// .java_pid<pid>. and .attach_pid<pid>. It is important that this | ||
// location is the same for all processes, otherwise the tools | ||
// will not be able to find all Hotspot processes. | ||
// This is intentionally not the same as java.io.tmpdir, since | ||
// the latter can be changed by the user. | ||
// Any changes to this needs to be synchronized with HotSpot. | ||
private static final String tmpdir; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we can't cache this any more? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks David for pointing this out. A different instance with a different PID may have a different temp path, so the path should be specific to the instance. Since we don't need the path outside the constructor, I think we don't need to cache it. On the other hand I see that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would save it in a local in the constructor and pass it to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
/** | ||
* HotSpot PerfData file prefix | ||
*/ | ||
private static final String HSPERFDATA_PREFIX = "hsperfdata_"; | ||
|
||
/** | ||
* Use platform specific methods for looking up temporary directories. | ||
*/ | ||
private static final PlatformSupport platformSupport = PlatformSupport.getInstance(); | ||
|
||
String socket_path; | ||
private OperationProperties props = new OperationProperties(VERSION_1); // updated in ctor | ||
|
||
|
@@ -67,10 +74,12 @@ public class VirtualMachineImpl extends HotSpotVirtualMachine { | |
// 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. | ||
File socket_file = new File(tmpdir, ".java_pid" + pid); | ||
// In macOS the socket file is located in per-user temp directory. | ||
String tempDir = getTempDirFromPid(pid); | ||
File socket_file = new File(tempDir, ".java_pid" + pid); | ||
socket_path = socket_file.getPath(); | ||
if (!socket_file.exists()) { | ||
File f = createAttachFile(pid); | ||
File f = createAttachFile(tempDir, pid); | ||
try { | ||
checkCatchesAndSendQuitTo(pid, false); | ||
|
||
|
@@ -211,12 +220,34 @@ protected void close(long fd) throws IOException { | |
} | ||
} | ||
|
||
private File createAttachFile(int pid) throws IOException { | ||
File f = new File(tmpdir, ".attach_pid" + pid); | ||
private File createAttachFile(String dir, int pid) throws IOException { | ||
File f = new File(dir, ".attach_pid" + pid); | ||
createAttachFile0(f.getPath()); | ||
return f; | ||
} | ||
|
||
/* | ||
* Returns a platform-specific temporary directory for a given process. | ||
* In VMs running as unprivileged user it returns the default platform-specific | ||
* temporary directory. In VMs running as root it searches over the list of | ||
* temporary directories for one containing HotSpot PerfData directory. | ||
*/ | ||
private String getTempDirFromPid(int pid) { | ||
ProcessHandle ph = ProcessHandle.of(pid).orElse(null); | ||
if (ph != null) { | ||
String user = ph.info().user().orElse(null); | ||
if (user != null) { | ||
for (String dir : platformSupport.getTemporaryDirectories(pid)) { | ||
Path fullPath = Path.of(dir, HSPERFDATA_PREFIX + user, String.valueOf(pid)); | ||
if (Files.exists(fullPath)) { | ||
return dir; | ||
} | ||
} | ||
} | ||
} | ||
return PlatformSupport.getTemporaryDirectory(); | ||
} | ||
|
||
//-- native methods | ||
|
||
static native boolean checkCatchesAndSendQuitTo(int pid, boolean throwIfNotReady) throws IOException, AttachNotSupportedException; | ||
|
@@ -235,10 +266,7 @@ private File createAttachFile(int pid) throws IOException { | |
|
||
static native void createAttachFile0(String path); | ||
|
||
static native String getTempDir(); | ||
|
||
static { | ||
System.loadLibrary("attach"); | ||
tmpdir = getTempDir(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is convenient but I'm not sure it is appropriate. Need the serviceability folk to comment.