Support JDK17 (msgpack#660)

xerial · web-flow · commit 117f1612fc3c · 2022-06-28T22:09:48.000+09:00
* Reproduce msgpack#600 (JDK17 error) * Add workaround for Java17 * Add a helpful error message * Add note on JDK17
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -26,14 +26,33 @@ jobs:
       - uses: actions/checkout@v2
       - name: jcheckstyle
         run: ./sbt jcheckStyle
+  test_jdk17:
+    name: Test JDK17
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions/setup-java@v3
+        with:
+          distribution: 'zulu'
+          java-version: '17'
+      - uses: actions/cache@v2
+        with:
+          path: ~/.cache
+          key: ${{ runner.os }}-jdk11-${{ hashFiles('**/*.sbt') }}
+          restore-keys: ${{ runner.os }}-jdk17-
+      - name: Test
+        run: ./sbt test
+      - name: Universal Buffer Test
+        run: ./sbt test -J-Dmsgpack.universal-buffer=true
   test_jdk11:
     name: Test JDK11
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - uses: olafurpg/setup-scala@v10
+      - uses: actions/setup-java@v3
         with:
-          java-version: adopt@1.11
+          distribution: 'zulu'
+          java-version: '11'
       - uses: actions/cache@v2
         with:
           path: ~/.cache
@@ -48,9 +67,10 @@ jobs:
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v2
-      - uses: olafurpg/setup-scala@v10
+      - uses: actions/setup-java@v3
         with:
-          java-version: adopt@1.8
+          distribution: 'zulu'
+          java-version: '8'
       - uses: actions/cache@v2
         with:
           path: ~/.cache
diff --git a/README.md b/README.md
@@ -42,6 +42,15 @@ dependencies {
 
 - [Usage examples](https://github.com/msgpack/msgpack-java/blob/develop/msgpack-core/src/test/java/org/msgpack/core/example/MessagePackExample.java)
 
+### Java 17 Support
+
+For using DirectByteBuffer (off-heap memory access methods) in JDK17, you need to specify two JVM options:
+```
+--add-opens=java.base/java.nio=ALL-UNNAMED
+--add-opens=java.base/sun.nio.ch=ALL-UNNAMED
+```
+
+
 ### Integration with Jackson ObjectMapper (jackson-databind)
 
 msgpack-java supports serialization and deserialization of Java objects through [jackson-databind](https://github.com/FasterXML/jackson-databind).
diff --git a/build.sbt b/build.sbt
@@ -74,6 +74,13 @@ lazy val msgpackCore = Project(id = "msgpack-core", base = file("msgpack-core"))
       "org.msgpack.value.impl"
     ),
     testFrameworks += new TestFramework("wvlet.airspec.Framework"),
+    Test / javaOptions ++= Seq(
+      // --add-opens is not available in JDK8
+      "-XX:+IgnoreUnrecognizedVMOptions",
+      "--add-opens=java.base/java.nio=ALL-UNNAMED",
+      "--add-opens=java.base/sun.nio.ch=ALL-UNNAMED"
+    ),
+    Test / fork := true,
     libraryDependencies ++= Seq(
       // msgpack-core should have no external dependencies
       junitInterface,
diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/DirectBufferAccess.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/DirectBufferAccess.java
@@ -18,19 +18,22 @@
 import java.lang.reflect.Constructor;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
+import java.nio.Buffer;
 import java.nio.ByteBuffer;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 
 import sun.misc.Unsafe;
+import sun.nio.ch.DirectBuffer;
 
 /**
  * Wraps the difference of access methods to DirectBuffers between Android and others.
  */
 class DirectBufferAccess
 {
     private DirectBufferAccess()
-    {}
+    {
+    }
 
     enum DirectBufferConstructorType
     {
@@ -40,7 +43,6 @@ enum DirectBufferConstructorType
         ARGS_MB_INT_INT
     }
 
-    static Method mGetAddress;
     // For Java <=8, gets a sun.misc.Cleaner
     static Method mCleaner;
     static Method mClean;
@@ -95,10 +97,19 @@ enum DirectBufferConstructorType
             if (byteBufferConstructor == null) {
                 throw new RuntimeException("Constructor of DirectByteBuffer is not found");
             }
-            byteBufferConstructor.setAccessible(true);
 
-            mGetAddress = directByteBufferClass.getDeclaredMethod("address");
-            mGetAddress.setAccessible(true);
+            try {
+                byteBufferConstructor.setAccessible(true);
+            }
+            catch (RuntimeException e) {
+                // This is a Java9+ exception, so we need to detect it without importing it for Java8 support
+                if ("java.lang.reflect.InaccessibleObjectException".equals(e.getClass().getName())) {
+                    byteBufferConstructor = null;
+                }
+                else {
+                    throw e;
+                }
+            }
 
             if (MessageBuffer.javaVersion <= 8) {
                 setupCleanerJava6(direct);
@@ -160,6 +171,7 @@ public Object run()
 
     /**
      * Checks if we have a usable {@link DirectByteBuffer#cleaner}.
+     *
      * @param direct a direct buffer
      * @return the method or an error
      */
@@ -184,6 +196,7 @@ private static Object getCleanerMethod(ByteBuffer direct)
 
     /**
      * Checks if we have a usable {@link sun.misc.Cleaner#clean}.
+     *
      * @param direct a direct buffer
      * @param mCleaner the {@link DirectByteBuffer#cleaner} method
      * @return the method or null
@@ -210,6 +223,7 @@ private static Object getCleanMethod(ByteBuffer direct, Method mCleaner)
 
     /**
      * Checks if we have a usable {@link Unsafe#invokeCleaner}.
+     *
      * @param direct a direct buffer
      * @return the method or an error
      */
@@ -218,7 +232,7 @@ private static Object getInvokeCleanerMethod(ByteBuffer direct)
         try {
             // See https://bugs.openjdk.java.net/browse/JDK-8171377
             Method m = MessageBuffer.unsafe.getClass().getDeclaredMethod(
-                "invokeCleaner", ByteBuffer.class);
+                    "invokeCleaner", ByteBuffer.class);
             m.invoke(MessageBuffer.unsafe, direct);
             return m;
         }
@@ -233,17 +247,9 @@ private static Object getInvokeCleanerMethod(ByteBuffer direct)
         }
     }
 
-    static long getAddress(Object base)
+    static long getAddress(Buffer buffer)
     {
-        try {
-            return (Long) mGetAddress.invoke(base);
-        }
-        catch (IllegalAccessException e) {
-            throw new RuntimeException(e);
-        }
-        catch (InvocationTargetException e) {
-            throw new RuntimeException(e);
-        }
+        return ((DirectBuffer) buffer).address();
     }
 
     static void clean(Object base)
@@ -253,7 +259,7 @@ static void clean(Object base)
                 Object cleaner = mCleaner.invoke(base);
                 mClean.invoke(cleaner);
             }
-        else {
+            else {
                 mInvokeCleaner.invoke(MessageBuffer.unsafe, base);
             }
         }
@@ -269,6 +275,10 @@ static boolean isDirectByteBufferInstance(Object s)
 
     static ByteBuffer newByteBuffer(long address, int index, int length, ByteBuffer reference)
     {
+        if (byteBufferConstructor == null) {
+            throw new IllegalStateException("Can't create a new DirectByteBuffer. In JDK17+, two JVM options needs to be set: " +
+                    "--add-opens=java.base/java.nio=ALL-UNNAMED and --add-opens=java.base/sun.nio.ch=ALL-UNNAMED");
+        }
         try {
             switch (directBufferConstructorType) {
                 case ARGS_LONG_INT_REF:
diff --git a/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala b/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala
@@ -0,0 +1,29 @@
+//
+// MessagePack for Java
+//
+//    Licensed under the Apache License, Version 2.0 (the "License");
+//    you may not use this file except in compliance with the License.
+//    You may obtain a copy of the License at
+//
+//        http://www.apache.org/licenses/LICENSE-2.0
+//
+//    Unless required by applicable law or agreed to in writing, software
+//    distributed under the License is distributed on an "AS IS" BASIS,
+//    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+//    See the License for the specific language governing permissions and
+//    limitations under the License.
+//
+package org.msgpack.core.buffer
+
+import wvlet.airspec.AirSpec
+
+import java.nio.ByteBuffer
+
+class DirectBufferAccessTest extends AirSpec {
+
+  test("instantiate DirectBufferAccess") {
+    val bb   = ByteBuffer.allocateDirect(1)
+    val addr = DirectBufferAccess.getAddress(bb)
+
+  }
+}
