changeset 2598:3d42591a926c

[PATCH] fix profiling and heap dump on Windows Both profiling and heap dump require the creation of files or directories within the user's temp directory (as defined by "java.io.tmp"( Current Thermostat code uses PosixFilePermissions, which fails hard at runtime on Windows. This patch removes the use of PosixFilePermissions on Windows only, allowing Heap Dump to work, and giving Profiling a better chance of working (more work is to be done there; I'm afraid that will be another patch) On Windows, the default temp directory is private to the user, and secure. Further patches will check if java.io.tmpdir is secure, and log a warning if it isn't, and probably allow the user to configure a global directory secured with an ACL if required to debug 'foreign' JVMs. (This means that further work will need to be done to connect to another user's JVM for some Thermostat features that rely on the temp directory) reviewed-by: sgehwolf review-thread: http://icedtea.classpath.org/pipermail/thermostat/2017-February/022241.html
author Simon Tooke <stooke@redhat.com>
date Thu, 23 Feb 2017 09:22:15 -0500
parents bfbb43dd6989
children 1652930da47b
files vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/HeapDump.java vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/InstrumentationControl.java
diffstat 2 files changed, 26 insertions(+), 7 deletions(-) [+]
line wrap: on
line diff
--- a/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/HeapDump.java	Wed Feb 22 17:56:51 2017 +0100
+++ b/vm-heap-analysis/common/src/main/java/com/redhat/thermostat/vm/heap/analysis/common/HeapDump.java	Thu Feb 23 09:22:15 2017 -0500
@@ -49,6 +49,7 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import com.redhat.thermostat.shared.config.OS;
 import org.apache.lucene.analysis.core.SimpleAnalyzer;
 import org.apache.lucene.document.Document;
 import org.apache.lucene.document.Field;
@@ -188,7 +189,13 @@
         String dirname = "thermostat-" + System.getProperty("user.name");
         File tmpFile = new File(System.getProperty("java.io.tmpdir"), dirname);
         if (! tmpFile.exists()) {
-            Files.createDirectory(tmpFile.toPath(), PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")));
+            if (OS.IS_UNIX) {
+                Files.createDirectory(tmpFile.toPath(), PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString("rwx------")));
+            }
+            else {
+                // windows Java doesn't support PosixFilePermissions class, but the tmpfir is private by default
+                Files.createDirectory(tmpFile.toPath());
+            }
             return tmpFile;
         } else {
             if (tmpFile.isDirectory()) {
--- a/vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/InstrumentationControl.java	Wed Feb 22 17:56:51 2017 +0100
+++ b/vm-profiler/jvm-agent/src/main/java/com/redhat/thermostat/vm/profiler/agent/jvm/InstrumentationControl.java	Thu Feb 23 09:22:15 2017 -0500
@@ -218,13 +218,25 @@
             return new ResultsFile(output);
         }
 
+        private boolean isPosixSystem() {
+            final String osname = System.getProperty("os.name").toLowerCase();
+            final boolean IS_UNIX = !osname.contains("win");
+            return IS_UNIX;
+        }
+
         private Path createOutput() throws IOException {
-            Set<PosixFilePermission> perm = PosixFilePermissions.fromString("rw-------");
-            FileAttribute<Set<PosixFilePermission>> attributes = PosixFilePermissions.asFileAttribute(perm);
-            // Include the pid so agent can find it. Surround pid with - to
-            // avoid false prefix-based matches. Otherwise the agent searching
-            // for "-12" may find "-123" as a valid match.
-            return Files.createTempFile("thermostat-" + getProcessId() + "-", ".perfdata", attributes);
+            if (isPosixSystem()) {
+                Set<PosixFilePermission> perm = PosixFilePermissions.fromString("rw-------");
+                FileAttribute<Set<PosixFilePermission>> attributes = PosixFilePermissions.asFileAttribute(perm);
+                // Include the pid so agent can find it. Surround pid with - to
+                // avoid false prefix-based matches. Otherwise the agent searching
+                // for "-12" may find "-123" as a valid match.
+                return Files.createTempFile("thermostat-" + getProcessId() + "-", ".perfdata", attributes);
+            }
+            else {
+                // should check that this is in a secure location
+                return Files.createTempFile("thermostat-" + getProcessId() + "-", ".perfdata");
+            }
         }
 
         private String getProcessId() {