error reporting path fix in java.util.prefs.FileSytemPreferences.lockFile0()

[Roger Riggs]

Hi Harry,

Great to see a new contributor.

If you haven't read up on the OpenJDK processes, there are a few things 
you can read
about how to contribute [1] and the developers guide [2]

Typically, changes are applied to the current developers release (JDK 9) 
and then backported
as needed. Please provide a description of the problem we can see if 
there is already a bugid for it
or create one.  Send the email to core-libs-dev at openjdk.java.net

If the patch is not too big you can include it in an email to 
core-libs-dev at openjdk.java.net
and folks will review it and someone will volunteer to sponsor it for you.
Changes typically require a test case so it can be shown that the test 
fails without the change
and works with the change.

Regards, Roger


[1] http://openjdk.java.net/contribute/
[2] http://openjdk.java.net/guide/

On 4/22/2016 12:53 PM, harry muttart wrote:
> Greetings,
>
> I would like to submit a jdk runtime fix on behalf of my employer, 
> Azul Systems.
>
> The issue is a long-standing error reporting corner case in the 
> openjdk source
> base.  The same error exists in jdk6, jdk7, jdk8, and jdk9 sources. 
> The same
> change easily applies to all of these source bases.  A recent check of 
> the latest
> openjdk source base shows this as still an issue.
>
> As I am a complete openjdk process newbie.  Can someone please recommend
> the simplest way to make this happen?
>
> Thanks to all for your patience.
>
> Harry Muttart
> Azul Systems
>
>
>
> The issue...
> ============
>
> From the runtime code below, you can see there is a path through
> java.util.prefs.FileSytemPreferences.lockFile0() which can result in an
> error return with an unset errno field.  On the path where the incoming
> argument, shared, is JNI_TRUE, any errno set by an open failure is not
> saved (in result[1]).  Then, when the return code from the open is 
> checked,
> the code zeroes result[0] but the errno slot remains unset. Because
> return[] is stack allocated, return[1] may contain ANY value, and the
> reported errno returned is completely unhelpful.
>
> There is an easy fix.  Capture the errno value after ALL fopen() calls.
>
> An Azul customer tripped this issue.
>
>
> The source file containing the "fix opportunity" is...
> jdk6, jdk7, jdk8)
>       jdk/src/solaris/native/java/util/FileSystemPreferences.c
> jdk9) jdk/src/java.prefs/unix/native/libprefs/FileSystemPreferences.c
>
>
> The problem code (from openjdk6 source base) is:
>
> Java_java_util_prefs_FileSystemPreferences_lockFile0(JNIEnv *env,
>         jclass thisclass, jstring java_fname, jint permission, 
> jboolean shared) {
>         const char *fname = JNU_GetStringPlatformChars(env, 
> java_fname, JNI_FALSE);
>         int fd, rc;
>         int result[2];
>         jintArray javaResult;
>         int old_umask;
>         FLOCK fl;
>
>         fl.l_whence = SEEK_SET;
>         fl.l_len = 0;
>         fl.l_start = 0;
>         if (shared == JNI_TRUE) {
>             fl.l_type = F_RDLCK;
>         } else {
>             fl.l_type = F_WRLCK;
>         }
>
>         if (shared == JNI_TRUE) {
>             fd = open(fname, O_RDONLY, 0);
> // NEED FIX HERE:  "result[1] = errno;"
>         } else {
>             old_umask = umask(0);
>             fd = open(fname, O_WRONLY|O_CREAT, permission);
>             result[1] = errno;
>             umask(old_umask);
>         }
>
>         if (fd < 0) {
>             result[0] = 0;
>         } else {
>             rc = fcntl(fd, F_SETLK64, &fl);
>             result[1] = errno;
>             if (rc < 0) {
>                 result[0]= 0;
>                 close(fd);
>             } else {
>               result[0] = fd;
>             }
>         }
>         JNU_ReleaseStringPlatformChars(env, java_fname, fname);
>         javaResult = (*env)->NewIntArray(env,2);
>         (*env)->SetIntArrayRegion(env, javaResult, 0, 2, result);
>         return javaResult;
>     }
>