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

Daniel D. Daugherty daniel.daugherty at oracle.com
Wed Jan 12 10:44:55 PST 2011

Kelly,

Coleen's fix is a partial anti-delta of the fix for 6938627:

src/os/linux/vm/os_linux.cpp

src/os/solaris/vm/os_solaris.cpp

src/os/windows/vm/os_windows.cpp

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