Thanks Marcus, I will fix those two issues before pushing.
/Staffan On 8 apr 2014, at 13:52, Markus Grönlund <markus.gronl...@oracle.com> wrote: > Hi Staffan, > > I think it looks good. > > Some minor things: > > src/share/native/sun/misc/VMSupport.c: > > line 60: There is some weirdness with the indentation. > > src/share/javavm/export/jvm.h (both Hotspot and JDK): > > You might want to consider co-locating > > 114 JNIEXPORT jstring JNICALL > 115 JVM_GetTemporaryDirectory(JNIEnv *env); > > with > > 1334 JNIEXPORT jobject JNICALL > 1335 JVM_InitAgentProperties(JNIEnv *env, jobject agent_props); > > > Other than that I thinks good - thanks for fixing this. > > /Markus > > > -----Original Message----- > From: Staffan Larsen > Sent: den 7 april 2014 21:19 > To: Staffan Larsen > Cc: serviceability-dev@openjdk.java.net serviceability-dev@openjdk.java.net; > hotspot-runtime-dev > Subject: Re: RFR: 8033104 sun/jvmstat/monitor/MonitoredVm/CR6672135.java > failing on all platforms > > And the links: > > bug: https://bugs.openjdk.java.net/browse/JDK-8033104 > webrev: http://cr.openjdk.java.net/~sla/8033104/webrev.00/ > > Sorry about that, > /Staffan > > On 7 apr 2014, at 20:08, Staffan Larsen <staffan.lar...@oracle.com> wrote: > >> >> The problem here is that the code for finding local VMs is not looking for >> the data in the correct place. >> >> When a JVM is started it will create the perf-data file in a user-specific >> directory inside /tmp (*). The code in the JDK (PerfDataFile.java) that >> lists all active JVMs looks for the user-specific directory inside >> java.io.tmpdir. If a user sets -Djava.io.tmpdir on the command line, the >> code in PerfDataFile will look in the wrong place. >> >> (*) It's a little bit more complex. /tmp is used on Linux and Solaris. On OS >> X and Windows, there are user-specific temp directories that should be used, >> and so the VM queries the OS for these paths. >> >> The solution would be for PerfDataFile to use the same locations as the VM >> creates them in. The simplest way to guarantee that the same directory is >> used is to ask the VM to provide the location. Thus I have introduced a new >> JVM_ function: JVM_GetTemporaryDirectory. >> >> (Since this change touches both hotspot and jdk repos I will submit the >> hotspot part first under a different bug id (provided that the review goes >> well)). >> >> The newly added test starts two VM with all possible combinations of setting >> and not setting java.io.tmpdir to verify that the mechanism is indeed not >> looking at that variable. I also removed an if-statement in BasicTests.java >> which would have found this issue a long time ago had it not been there. >> >> Thanks, >> /Staffan >
