Bug 235018 - java/openjdk8: adding millisecond resolution to get/setLastModified breaks many apps
Summary: java/openjdk8: adding millisecond resolution to get/setLastModified breaks ma...
Status: Closed Works As Intended
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-java (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-17 07:15 UTC by aryeh.friedman
Modified: 2019-01-18 13:01 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (java)


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description aryeh.friedman 2019-01-17 07:15:43 UTC
The first diff in files/patch-jdk-src-solaris-native-java-io-UnixFileSystem_md.c:

--- jdk/src/solaris/native/java/io/UnixFileSystem_md.c.orig     2018-12-12 23:07:51.229721000 +0100
+++ jdk/src/solaris/native/java/io/UnixFileSystem_md.c  2018-12-12 23:12:21.847169000 +0100
@@ -208,7 +208,8 @@
     WITH_FIELD_PLATFORM_STRING(env, file, ids.path, path) {
         struct stat64 sb;
         if (stat64(path, &sb) == 0) {
-            rv = 1000 * (jlong)sb.st_mtime;
+            rv  = (jlong)sb.st_mtim.tv_sec * 1000;
+            rv += (jlong)sb.st_mtim.tv_nsec / 1000000;
         }
     } END_PLATFORM_STRING(env, path);
     return rv;

Breaks many applications that depend on second vs. millisecond resolution... for example the Last-modified header in tomcat is now incorrect and in a REST-like API/DB it leads the header and actual file time not being equal when they are in fact equal without the above patch.

This fundamentally breaks Java's promise of "Compile one, run anywhere"
Comment 1 aryeh.friedman 2019-01-17 07:22:07 UTC
Just a note the HTTP spec (RFC 2616 section 13.3.2) specifies all times are in second resolution: "where the one-second resolution of HTTP"
Comment 2 Michael Osipov 2019-01-17 10:26:01 UTC
This affects java.io.UnixFileSystem and probably java.io.File. Well, File#lastModified() has millisecond precision. Looking at the code in Java 11 you see:

> JNIEXPORT jlong JNICALL
> Java_java_io_UnixFileSystem_getLastModifiedTime(JNIEnv *env, jobject this,
>                                                 jobject file)
> {
>     jlong rv = 0;
> 
>     WITH_FIELD_PLATFORM_STRING(env, file, ids.path, path) {
>         struct stat64 sb;
>         if (stat64(path, &sb) == 0) {
> #if defined(_AIX)
>             rv =  (jlong)sb.st_mtime * 1000;
>             rv += (jlong)sb.st_mtime_n / 1000000;
> #elif defined(MACOSX)
>             rv  = (jlong)sb.st_mtimespec.tv_sec * 1000;
>             rv += (jlong)sb.st_mtimespec.tv_nsec / 1000000;
> #else
>             rv  = (jlong)sb.st_mtim.tv_sec * 1000;
>             rv += (jlong)sb.st_mtim.tv_nsec / 1000000;
> #endif
>         }
>     } END_PLATFORM_STRING(env, path);
>     return rv;
> }

This is the default now.

How does it break Tomcat, I don't get it? The RFC you are quoting is invalid. Look into RFC 7231, chapter 7.1.1.
Comment 3 aryeh.friedman 2019-01-17 17:44:22 UTC
Java 8 != Java 11 

Java makes the absolute guarantee that if you compile for one language spec version it will work on all JVM's on all platforms of the same language spec version.  Changing the granularity of a method fundamentally breaks that promise.  The original source for OpenJDK8 is 1 second resolution and people do count on that.

Additionally RFC 7231 section 7.1.1.1 (as "imported" from RFC 5322, the provided EBNF does not include sub-second resolutions) specifies only second resolution it makes no provisions for having/not having sub-second resolutions (i.e. that is undefined behavior which again violates Java's portability promise if a specific JVM on a specific platform [OpenJDK 8 on FreeBSD] decides to arbitrarily change how one of the standard library's computes/returns values).

The way it can break a tomcat-based web app is that one of the practices used in some REST-like API's (such as the one that triggered this bug report) is that when a PUT/POST is made it is required to be proceeded by a GET, and then the PUT/POST request contains an If-Unmodified-Since header echoing back the Last-Modified header of the response to proceeding GET, to help minimize concurrent update issues, consistent with RFC-based standards.   The server's processing of the PUT/POST then involves comparing the re-sent header to the actual file modification time.   Since the HTTP standard time format for these headers is second resolution but the Java 11 (but NOT Java 8) standard library one is millisecond resolution, they almost never agree and therefore the PUT/POST is rejected as being out of date.

In a tomcat-based web app using Java 11, this problem could be solved by rounding down the time to a one-second resolution.  However, this step should not be necessary in a Java 8 application, because of Java's portability promise within any given language spec version.

Here is the specific line that fails:

if ( ! ifUnmodifiedSinceTime.equals(lastModifiedTime) )
                        throw new HResVersionConflictException(
                                   "Write conflict:  Up-to-date"
                                           + " If-Unmodified-Since"
                                           + " timestamp must be"
                                           + " obtained, as"
                                           + " Last-Modified"
                                           + " timestamp metadata,"
                                           + " via GET "
                                           + writeID.toUrlPath(
                                                      pathPrefix));

As proven by following debugging output:

-----------------------------------
MethodProcUtil.checkWriteConflict:
  -- ifUnmodifiedSinceTime = 1547742518000 | Thu Jan 17 11:28:38 EST 2019
  -- lastModifiedTime = 1547742518769 | Thu Jan 17 11:28:38 EST 2019
-----------------------------------

Here is the same debugging output from a pre-patch openjdk 8 on same version of FreeBSD (identical VM's running on the same host machine except that one VM uses the post-patch openjdk and the other VM uses pre-patched):

-----------------------------------
MethodProcUtil.checkWriteConflict:
  -- ifUnmodifiedSinceTime = 1546916804000 | Mon Jan 07 22:06:44 EST 2019
  -- lastModifiedTime = 1546916804000 | Mon Jan 07 22:06:44 EST 2019
-----------------------------------

Other factors being equal, millisecond resolution would, of course, be better than second resolution.  But the move to millisecond resolution should not break existing applications that depend on a particular version of Java.

If, for whatever reason, it is deemed necessary to take such a radical step as to make such an improvement without waiting for the appropriate version of Java, then any such change should AT LEAST be documented MUCH, MUCH more conspicuously than this one was.  (I found no documentation of this change at all except for a comment within the commit.)
Comment 4 aryeh.friedman 2019-01-18 03:34:52 UTC
The log files attached to the following minecraft server crash report appear to point to the same issue this bug report is concerned with and the same root cause.   Therefore I am changing the "importance" of this bug report from "affects only me" to "affects some people":

https://docs.freebsd.org/cgi/getmsg.cgi?fetch=4076+0+current/freebsd-java
Comment 5 aryeh.friedman 2019-01-18 03:43:04 UTC
Oops looks like I misread the stack trace on that email... it does point to a timing issue but since locks and file stat use different mechanisms is likely unrelated.
Comment 6 Michael Osipov 2019-01-18 12:04:54 UTC
I think we need to break things down here because here we have several issues mixed:

1. Yes, I do agree that this change is wrong because the code isn't maintain and not portable. Moreover, this isn't something platform-specific. I have found the changes in Java 10 for millisecond precision, but Java 8 shall remain as-is: http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/f0b93fbd8cf8/src/solaris/native/java/io/UnixFileSystem_md.c
2. You are application is flawed (ifUnmodifiedSinceTime.equals(lastModifiedTime)) and you are relying on an implementation detail instead on the documentation:

> The Javadoc clearly says: A long value representing the time the file was last  modified, measured in milliseconds since the epoch (00:00:00 GMT, January 1,  1970), or 0L if the file does not exist or if an I/O error occurs

It may or may not contain millisecond precision, but the entire value is milliseconds. HTTP dates values are in second resolution, yet you convert it to a field with millisecond resolution. You must truncate both values down to seconds to make it comparable. Everything else is just flawed. If you would rely on NIO2 or new date time API with Instants you'd have the same issue in Java 8.
Comment 7 Alex Dupre freebsd_committer freebsd_triage 2019-01-18 12:48:25 UTC
I'm sorry about your breakage, but you are confusing the java spec with a specific implementation on a specific OS. Both Java 8 and Java 11 have the same spec in relation to the getLastModifiedTime resolution, and there are already implementations on different OSes / filesystems / JREs that returns the correct/documented resolution, so every application that fails because it blindly ignores the milliseconds is simply flawed, included tomcat if it compares a millisecond value with a second value multiplied by 1000 instead of the opposite.
Comment 8 Michael Osipov 2019-01-18 12:50:58 UTC
(In reply to Alex Dupre from comment #7)

Thanks for confirming my statement. That's pretty much it, though I don't expect anyone to change such things.
Comment 9 Alex Dupre freebsd_committer freebsd_triage 2019-01-18 13:01:42 UTC
For instance, java 8 on windows returns the millisecond resolution.