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

[David Holmes]

So I understand that various tools will break because the tool process 
puts a file in a "tmp" location for the target VM to see, under the 
assumption that they both have the same notion of what "tmp" is.

My question is do we have a clear specification for what things in the 
JDK (the user often can't tell the difference between VM usage and 
library usage) will be affected by java.io.tmpdir, and what will use 
some hard-wired notion of "tmp" ?

David

Coleen Phillimore said the following on 01/13/11 05:19:
> I put (S) in the title but it meant small, not simple.   More below.
> 
> On 1/12/2011 1:29 PM, 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. :^(
> 
> I'm not surprised VS2010 compilers don't work without TEMP or TMP 
> defined.  I don't think a lot of things would work on windows.  I'll 
> test it out on my machine and see what sort of error we get, but we 
> really only use get_temp_directory() for a couple things that have 
> backoffs if there's an error opening the file.  Also if GetTempPath 
> returns null we return "", which should use the current working 
> directory (except one case in hs_err, I added a path separator, which I 
> should only conditionally add).
> 
>>
>> 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?
> I think TMPDIR can be a user environment variable too, and we don't want 
> user environment variables.  Systems without /tmp or define tmp to be 
> something else can be problematic, which is why we had changed it to use 
> java.io.tmpdir.  But this broke a lot of other things.  When we port to 
> said systems with different tmps we can fix the code for these then with 
> some #ifdefs or whatever.
> 
>>
>> 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?
> It does actually matter  because /tmp is apparently faster on solaris 
> and using java.io.tmpdir as /var/tmp could slow down perfdata.  There's 
> a lot of discussion in these bugs (the one I pointed to and also the 
> see-also bugs).  The code that tries to create files based on 
> non-existent temp directories will fail.  We do have failure messages 
> for those in all cases.
> 
>>
>> I know this seems like a simple change, but I'm not so sure it is that 
>> simple.
> No, I agree it's not a simple matter, which is why it took a while to 
> decide to revert the change we made.
>>
>> 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?
> 
> When the VM uses /tmp or %TEMP% (or %TMP%) it puts the process id in the 
> file because it's assumed to be in a  common place with other java 
> processes running.  The property java.io.tmpdir allows users to write 
> Java code that doesn't have to make that assumption, and that is why 
> this is breaking.  Apparently  (according to the bug report) running 
> Tomcat with java.io.tmpdir is necessary because of the tomcat temporary 
> files.  The VM's temporary files are going to be different.  They need 
> to be per-system.
> 
> I don't really know if hardcoding /tmp is a good idea or not but that's 
> what we've had before and changing it is going to cause problems for 
> people that they will (have already) report as regressions.   This 
> change changes it back to what we had.
> 
> thanks,
> Coleen
> 
> 
>>
>> -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
>>>
>>
>