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

Kelly O'Hair kelly.ohair at oracle.com
Wed Jan 12 10:29:52 PST 2011

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 gettempdirectory() 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 gettempdirectory() 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/viewbug.do?bugid=7009828 Thanks, Coleen

More information about the hotspot-runtime-dev mailing list