Re: [PATCH] Prefer TMPDIR over hard coded /tmp

[Robert Stupp]

Right, it's come a few times. In principle it seems reasonable but it 
will likely have a tail of issues. One concern is that it will cause 
issues with the attach mechanism as that depends on the tool side 
knowing the target VM's tmp dir. There's also the transition issue 
where old JDKs will be using /tmp, new JDKs using whatever TMPDIR is 
set too. This is one of these issues where most of the work will be 
shaking out the knock on impact (that will be needed for the CSR and 
release note at least).


I'm not sure, whether I understand the concern about the attach 
mechanism, as that seems to use the directory returned by 
PlatformSupport->VMSupport.getVMTemporaryDirectory->JVM_GetTemporaryDirectory->os::get_temp_directory, 
which returns the hard coded "/tmp" for all Unix platforms (except 
MacOS). (No intention to change this.)


The concern about backward compatibility (app using a JVM with hard 
coded /tmp plus a JVM using TMPDIR) is absolutely correct. I'm just not 
sure whether that's actually (or usually) an issue for /tmp specifically 
(at least on Linux, 
http://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch03s18.html). Well, at 
least if people actually use /tmp as it's supposed to be used, since 
/tmp could perfectly be a volatile filesystem, but it's actually on 
persistent storage, probably due to "misuse".


What I'm currently thinking of is, whether a JVM flag (-XX:+PreferTMPDIR 
or so, defaults to false) could be a compromise that preserves the 
current behavior and prevents issues with existing applications but 
gives people the option to use TMPDIR for java.io.tmpdir, if they want 
to. Using TMPDIR for java.io.tmpdir is mostly useful for building and 
testing code - i.e. having an easier way to use different locations for 
java.io.tmpdir instead of, for example, digging through build files and 
inspecting command lines to check, whether surefire/junit/testng/JMH 
passed the correct java.io.tmpdir properly to a spawned, maybe short 
lived, child JVMs. WDYT?


Robert

--
Robert Stupp
@snazy

Re: [PATCH] Prefer TMPDIR over hard coded /tmp

[Alan Bateman]

On 26/04/2018 01:45, Brian Burkhalter wrote:

On Apr 25, 2018, at 5:38 PM, Brian Burkhalter  
wrote:


The attached patch changed the behavior to use the content of the environment 
variable TMPDIR, if present. Note that this does not change where the hsperf* 
folders are created.

The source change looks OK to me aside from the copyright year and that in the 
second line here

Note that I am only saying that the patch looks OK, not that we want to use it. 
There have been a number of issues filed in this area over time, for example 
https://bugs.openjdk.java.net/browse/JDK-6205979. There are arguments against 
this change.

Right, it's come a few times. In principle it seems reasonable but it 
will likely have a tail of issues. One concern is that it will cause 
issues with the attach mechanism as that depends on the tool side 
knowing the target VM's tmp dir. There's also the transition issue where 
old JDKs will be using /tmp, new JDKs using whatever TMPDIR is set too. 
This is one of these issues where most of the work will be shaking out 
the knock on impact (that will be needed for the CSR and release note at 
least).


-Alan

Re: [PATCH] Prefer TMPDIR over hard coded /tmp

[Bernd Eckenfels]

Hello.

For robustness reasons, should it maybe fall back to the hardcoded default if 
the target path does not exist or has the (obvious*) missing write permission?

Gruss
Bernd

* with or without trying a actual write?
--
http://bernd.eckenfels.net
_
From: Brian Burkhalter <brian.burkhal...@oracle.com>
Sent: Donnerstag, April 26, 2018 5:09 AM
Subject: Re: [PATCH] Prefer TMPDIR over hard coded /tmp
To: Robert Stupp <sn...@snazy.de>
Cc: <core-libs-dev@openjdk.java.net>


Hi Robert,

On Apr 23, 2018, at 7:23 AM, Robert Stupp <sn...@snazy.de> wrote:

> For MacOS and Windows, Java prefers the user's temporary directory for 
> java.io.tmpdir, but not for Linux, where it is always set to /tmp. The burden 
> with this is that if you want to use a different temp directory, you have to 
> explicitly pass -Djava.io.tmpdir=... on the command line, which can be 
> difficult especially for unit tests or microbenchmarks in 3rd party code ran 
> via Maven for example.
>
> java_props_t.tmp_dir is always initialized as P_tmpdir in GetJavaProperties 
> (src/java.base/unix/native/libjava/java_props_md.c).
>
> The attached patch changed the behavior to use the content of the environment 
> variable TMPDIR, if present. Note that this does not change where the hsperf* 
> folders are created.

The source change looks OK to me aside from the copyright year and that in the 
second line here

+ v = getenv("TMPDIR");
+ if (v) {
+ sprops.tmp_dir = strdup(v);
+ }

it should probably have

+ if (v != NULL) {

for consistency with elsewhere in the file.

> I'm not sure why java.io.tmpdir is always /tmp in Linux as according to the 
> SCM history, this was always as it is (references the initial OpenJDK commit).
>
> I haven't found any tests that validates the content of java.io.tmpdir.

Some sort of test would need to be added to this patch to validate the change.

Thanks,

Brian

Re: [PATCH] Prefer TMPDIR over hard coded /tmp

[Brian Burkhalter]

On Apr 25, 2018, at 5:38 PM, Brian Burkhalter  
wrote:

>> The attached patch changed the behavior to use the content of the 
>> environment variable TMPDIR, if present. Note that this does not change 
>> where the hsperf* folders are created.
> 
> The source change looks OK to me aside from the copyright year and that in 
> the second line here

Note that I am only saying that the patch looks OK, not that we want to use it. 
There have been a number of issues filed in this area over time, for example 
https://bugs.openjdk.java.net/browse/JDK-6205979. There are arguments against 
this change.

Brian

Re: [PATCH] Prefer TMPDIR over hard coded /tmp

[Brian Burkhalter]

Hi Robert,

On Apr 23, 2018, at 7:23 AM, Robert Stupp  wrote:

> For MacOS and Windows, Java prefers the user's temporary directory for 
> java.io.tmpdir, but not for Linux, where it is always set to /tmp. The burden 
> with this is that if you want to use a different temp directory, you have to 
> explicitly pass -Djava.io.tmpdir=... on the command line, which can be 
> difficult especially for unit tests or microbenchmarks in 3rd party code ran 
> via Maven for example.
> 
> java_props_t.tmp_dir is always initialized as P_tmpdir in GetJavaProperties 
> (src/java.base/unix/native/libjava/java_props_md.c).
> 
> The attached patch changed the behavior to use the content of the environment 
> variable TMPDIR, if present. Note that this does not change where the hsperf* 
> folders are created.

The source change looks OK to me aside from the copyright year and that in the 
second line here

+v = getenv("TMPDIR");
+if (v) {
+sprops.tmp_dir = strdup(v);
+}

it should probably have

+if (v != NULL) {

for consistency with elsewhere in the file.

> I'm not sure why java.io.tmpdir is always /tmp in Linux as according to the 
> SCM history, this was always as it is (references the initial OpenJDK commit).
> 
> I haven't found any tests that validates the content of java.io.tmpdir.

Some sort of test would need to be added to this patch to validate the change.

Thanks,

Brian

[PATCH] Prefer TMPDIR over hard coded /tmp

[Robert Stupp]

For MacOS and Windows, Java prefers the user's temporary directory for 
java.io.tmpdir, but not for Linux, where it is always set to /tmp. The 
burden with this is that if you want to use a different temp directory, 
you have to explicitly pass -Djava.io.tmpdir=... on the command line, 
which can be difficult especially for unit tests or microbenchmarks in 
3rd party code ran via Maven for example.


java_props_t.tmp_dir is always initialized as P_tmpdir in 
GetJavaProperties (src/java.base/unix/native/libjava/java_props_md.c).


The attached patch changed the behavior to use the content of the 
environment variable TMPDIR, if present. Note that this does not change 
where the hsperf* folders are created.


I'm not sure why java.io.tmpdir is always /tmp in Linux as according to 
the SCM history, this was always as it is (references the initial 
OpenJDK commit).


I haven't found any tests that validates the content of java.io.tmpdir.

Robert

--
Robert Stupp
@snazy

# HG changeset patch
# User Robert Stupp 
# Date 1524398509 -7200
#  Sun Apr 22 14:01:49 2018 +0200
# Branch use-TMPDIR-env-Linux
# Node ID 3f9f58a5d4049fcba8e5201e321bf71984430ce9
# Parent  fcd5df7aa235ca39852b04eb589e25f156870ce4
Use TMPDIR environment variable for java.io.tmpdir on Linux

diff --git a/src/java.base/unix/native/libjava/java_props_md.c 
b/src/java.base/unix/native/libjava/java_props_md.c
--- a/src/java.base/unix/native/libjava/java_props_md.c
+++ b/src/java.base/unix/native/libjava/java_props_md.c
@@ -358,7 +358,7 @@
 return 
 }
 
-/* tmp dir */
+/* tmp dir, use the default from _PATH_VARTMP */
 sprops.tmp_dir = P_tmpdir;
 #ifdef MACOSX
 /* darwin has a per-user temp dir */
@@ -367,6 +367,14 @@
 if (pathSize > 0 && pathSize <= PATH_MAX) {
 sprops.tmp_dir = tmp_path;
 }
+#else
+/* Use the temporary directory injected via the environment variable TMPDIR
+ * instead of the "hard coded" _PATH_VARTMP
+ */
+v = getenv("TMPDIR");
+if (v) {
+sprops.tmp_dir = strdup(v);
+}
 #endif /* MACOSX */
 
 /* Printing properties */