Request for review (S) 7009828: Fix for 6938627 breaks visualvm monitoring when -Djava.io.tmpdir is defined

[Coleen Phillimore]

Thank you, Dan.  That was right, I'd added a '/' in places because with 
java.io.tmpdir 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 http://cr.openjdk.java.net/~coleenp/7009828_2/

Thanks,
Coleen

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 "java.io.tmpdir" 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 java.io.tmpdir 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 java.io.tmpdir 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 java.io.tmpdir breaks a lot of things.
>>>
>>> open webrev at http://cr.openjdk.java.net/~coleenp/7009828/
>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=7009828
>>>
>>> Thanks,
>>> Coleen
>>>
>>