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

[Daniel D. Daugherty]

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
>>
>