Hi Staffan - It looks good to me. Why is the bug marked "closed" though?
On Fri, Apr 25, 2014 at 8:56 AM, Staffan Larsen <staffan.lar...@oracle.com>wrote: > Still looking for a Review of this change. > > Thanks, > /Staffan > > On 7 apr 2014, at 21:19, Staffan Larsen <staffan.lar...@oracle.com> wrote: > > > 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 > > > > -- [image: twitter-icon-large.png] Keith McGuigan @kamggg kmcgui...@twitter.com