From 65188edfcb7a87f17dc1fdd5b5017ec555f4d3c5 Mon Sep 17 00:00:00 2001 From: "Taro L. Saito" Date: Tue, 28 Jun 2022 17:21:11 +0900 Subject: [PATCH 1/4] Reproduce #600 (JDk17 error) --- .github/workflows/CI.yml | 28 +++++++++++++++--- .../core/buffer/DirectBufferAccessTest.scala | 29 +++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index f289bc70..f845825e 100644 --- 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/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala b/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala new file mode 100644 index 00000000..ad38066a --- /dev/null +++ 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 arr = Array[Byte](0) + val bb = ByteBuffer.wrap(arr) + val addr = DirectBufferAccess.getAddress(bb) + } +} From 3dc7e9dde81824d58e9aff3bafa68ed8c93e1370 Mon Sep 17 00:00:00 2001 From: "Taro L. Saito" Date: Tue, 28 Jun 2022 18:25:20 +0900 Subject: [PATCH 2/4] Add workaround for Java17 --- build.sbt | 7 ++++ .../core/buffer/DirectBufferAccess.java | 40 +++++++++++-------- .../core/buffer/DirectBufferAccessTest.scala | 4 +- 3 files changed, 32 insertions(+), 19 deletions(-) diff --git a/build.sbt b/build.sbt index 274545e2..0154bf5a 100644 --- 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 index cde2e6ec..31232231 100644 --- 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,11 +18,13 @@ 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. @@ -30,7 +32,8 @@ 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); } } 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 index ad38066a..40f4c770 100644 --- a/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala +++ b/msgpack-core/src/test/scala/org/msgpack/core/buffer/DirectBufferAccessTest.scala @@ -22,8 +22,8 @@ import java.nio.ByteBuffer class DirectBufferAccessTest extends AirSpec { test("instantiate DirectBufferAccess") { - val arr = Array[Byte](0) - val bb = ByteBuffer.wrap(arr) + val bb = ByteBuffer.allocateDirect(1) val addr = DirectBufferAccess.getAddress(bb) + } } From fdb537a28129ef485675e516ced45e22a788597a Mon Sep 17 00:00:00 2001 From: "Taro L. Saito" Date: Tue, 28 Jun 2022 18:35:30 +0900 Subject: [PATCH 3/4] Add a helpful error message --- .../main/java/org/msgpack/core/buffer/DirectBufferAccess.java | 4 ++++ 1 file changed, 4 insertions(+) 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 index 31232231..f54c50b9 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/DirectBufferAccess.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/DirectBufferAccess.java @@ -275,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: From ecb4951a8fc1642c97c0545a0fcb1a732a9d1432 Mon Sep 17 00:00:00 2001 From: "Taro L. Saito" Date: Tue, 28 Jun 2022 22:08:33 +0900 Subject: [PATCH 4/4] Add note on JDK17 --- README.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/README.md b/README.md index 4035fdc4..7723eb6e 100644 --- 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).