Request for review (S) 7009828: Fix for 6938627 breaks visualvm monitoring when is defined

Coleen Phillimore coleen.phillimore at
Wed Jan 12 12:57:49 PST 2011

Thank you, Dan.  That was right, I'd added a '/' in places because with you could specify a path not ending in a separator.  I 
had to change this webrev a little bit for hs_err_pid file.  If on 
windows get_temp_directory() returns "" we don't try to create a file as 
"\hs_err_pid%p.log".  See:

open webrev at


On 1/12/2011 1:44 PM, Daniel D. Daugherty wrote:
> Kelly,
> Coleen's fix is a partial anti-delta of the fix for 6938627:
> src/os/linux/vm/os_linux.cpp
>   - previously returned "/tmp/"; now returns "/tmp", but the assumed
>     path separator was obsoleted by other changes made in 6938627
>     that have _not_ been anti-delta'ed
> src/os/solaris/vm/os_solaris.cpp
>   - previously returned "/tmp/"; now returns "/tmp", but the assumed
>     path separator was obsoleted by other changes made in 6938627
>     that have _not_ been anti-delta'ed
> src/os/windows/vm/os_windows.cpp
>   - removal of query of "" property restores the
>     behavior before 6938627
> I'm thumbs up on this partial anti-delta!
> Dan
> On 1/12/2011 11:29 AM, Kelly O'Hair wrote:
>> I'm a little concerned about this change.
>> On windows, this relies on GetTempPath, which seems to be influenced 
>> by the environment
>> variables TEMP or TMP. So first question is, what happens if TEMP or 
>> TMP are both undefined
>> or they refer to a directory that does not exist?
>> I recently discovered that the VS2010 compiler just doesn't work if 
>> TMP or TEMP are not defined properly,
>> and with very little hints as to why.  There is no guarantee that it 
>> doesn't just return 0 or a path that does
>> not exist. It can fail and then you have to call GetLastError to find 
>> out why, most people don't. :^(
>> On Solaris/Linux, this just hardwires /tmp, which of course should 
>> always exist, but it is not influenced
>> by any environment variables like TMPDIR. I'm not sure what the rules 
>> should be about obeying the value
>> of TMPDIR. And as I recall, TMPDIR is supposed to be a system wide 
>> temp area replacement for /tmp
>> on Solaris at least. Do we care about TMPDIR? Do we want to allow 
>> people to re-direct the temp directory
>> location?
>> Should the selection of the VM temp directory be influenced by 
>> environment variables?
>> Or is hardwiring to /tmp a good idea?
>> Should the temp directory be verified as far as existence and 
>> permissions?
>> The is /var/tmp on Linux and /tmp on Solaris as I 
>> recall, does that matter?
>> I know this seems like a simple change, but I'm not so sure it is 
>> that simple.
>> When I run tests I often re-define so that I don't 
>> collide with anyone else running tests
>> on the same system. Now I can't. You can argue the testcase is bad if 
>> it doesn't insure it creates unique
>> filenames in /tmp, so I won't use that as an argument to not change 
>> things. But I wonder how sloppy
>> we are in creating files in /tmp, are all uses making sure that they 
>> are unique and not used by anyone else?
>> -kto
>> On Jan 12, 2011, at 9:38 AM, Coleen Phillimore wrote:
>>> Summary: Change get_temp_directory() back to /tmp and %TEMP% like it 
>>> always was and where the tools expect it to be.
>>> There is more information in the bug, but essentially changing 
>>> get_temp_directory() to use breaks a lot of things.
>>> open webrev at
>>> bug link at
>>> Thanks,
>>> Coleen

More information about the hotspot-runtime-dev mailing list