From 40fea897fbf06920053f27c517ed7a5c50f0c5a2 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 13 Sep 2019 21:09:34 +0200 Subject: [PATCH 1/9] Improves MessageBuffer support of Java 11 Switches the MessageBuffer implementation to MessageBufferU for Java versions at least 9. Since this implementation overrides almost all MessageBuffer's method it is safe to allow direct buffers as argument to MessageBuffer.wrap(ByteBuffer) --- .../msgpack/core/buffer/MessageBuffer.java | 51 +++++++++++++------ .../msgpack/core/buffer/MessageBufferU.java | 10 ++++ 2 files changed, 45 insertions(+), 16 deletions(-) diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java index c85680905..a657e57e9 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java @@ -70,19 +70,7 @@ public class MessageBuffer try { // Check java version - String javaVersion = System.getProperty("java.specification.version", ""); - int dotPos = javaVersion.indexOf('.'); - boolean isJavaAtLeast7 = false; - if (dotPos != -1) { - try { - int major = Integer.parseInt(javaVersion.substring(0, dotPos)); - int minor = Integer.parseInt(javaVersion.substring(dotPos + 1)); - isJavaAtLeast7 = major > 1 || (major == 1 && minor >= 7); - } - catch (NumberFormatException e) { - e.printStackTrace(System.err); - } - } + int javaVersion = getJavaVersion(); boolean hasUnsafe = false; try { @@ -97,12 +85,14 @@ public class MessageBuffer // Is Google App Engine? boolean isGAE = System.getProperty("com.google.appengine.runtime.version") != null; - // For Java6, android and JVM that has no Unsafe class, use Universal MessageBuffer + // For Java6, android and JVM that has no Unsafe class, use Universal MessageBuffer (based on ByteBuffer). + // Java9 onward doesn't allow to access Cleaner by reflection, so we use Universal MessageBuffer again. useUniversalBuffer = Boolean.parseBoolean(System.getProperty("msgpack.universal-buffer", "false")) || isAndroid || isGAE - || !isJavaAtLeast7 + || javaVersion < 7 + || javaVersion > 9 || !hasUnsafe; if (!useUniversalBuffer) { @@ -175,6 +165,30 @@ public class MessageBuffer } } + static int getJavaVersion() { + String javaVersion = System.getProperty("java.specification.version", ""); + int dotPos = javaVersion.indexOf('.'); + if (dotPos != -1) { + try { + int major = Integer.parseInt(javaVersion.substring(0, dotPos)); + int minor = Integer.parseInt(javaVersion.substring(dotPos + 1)); + return major > 1 ? major : minor; + } + catch (NumberFormatException e) { + e.printStackTrace(System.err); + } + } + else { + try { + return Integer.parseInt(javaVersion); + } + catch (NumberFormatException e) { + e.printStackTrace(System.err); + } + } + return 6; + } + /** * Base object for resolving the relative address of the raw byte array. * If base == null, the address value is a raw memory address @@ -366,7 +380,12 @@ else if (DirectBufferAccess.isDirectByteBufferInstance(buffer.reference)) { { if (bb.isDirect()) { if (isUniversalBuffer) { - throw new UnsupportedOperationException("Cannot create MessageBuffer from a DirectBuffer on this platform"); + // MessageBufferU overrides almost all methods, only field 'size' is used. + this.base = null; + this.address = 0; + this.size = bb.remaining(); + this.reference = null; + return; } // Direct buffer or off-heap memory this.base = null; diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java index 4369bdf3c..7f76a5a17 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java @@ -258,4 +258,14 @@ public byte[] toByteArray() getBytes(0, b, 0, b.length); return b; } + + @Override + public boolean hasArray() { + return !wrap.isDirect(); + } + + @Override + public byte[] array() { + return hasArray() ? wrap.array() : null; + } } From 89d696416fd5c55624ea9ada3f579406f1f67594 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 13 Sep 2019 21:29:30 +0200 Subject: [PATCH 2/9] Corrects code style --- .../main/java/org/msgpack/core/buffer/MessageBuffer.java | 3 ++- .../main/java/org/msgpack/core/buffer/MessageBufferU.java | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java index a657e57e9..2521f8f66 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java @@ -165,7 +165,8 @@ public class MessageBuffer } } - static int getJavaVersion() { + static int getJavaVersion() + { String javaVersion = System.getProperty("java.specification.version", ""); int dotPos = javaVersion.indexOf('.'); if (dotPos != -1) { diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java index 7f76a5a17..a185a67b9 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBufferU.java @@ -260,12 +260,14 @@ public byte[] toByteArray() } @Override - public boolean hasArray() { + public boolean hasArray() + { return !wrap.isDirect(); } @Override - public byte[] array() { + public byte[] array() + { return hasArray() ? wrap.array() : null; } } From 77a76a2ba3d6d594566fd842f2b9b8f714881585 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Fri, 8 Nov 2019 23:55:54 +0100 Subject: [PATCH 3/9] Do not switch to MessageBufferU on Java 9+ Disables the automatic switch to MessageBufferU on Java 9+, falling back to a manual switch through Java properties. --- .../src/main/java/org/msgpack/core/buffer/MessageBuffer.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java index 2521f8f66..8b7f8d2f4 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java @@ -86,13 +86,11 @@ public class MessageBuffer boolean isGAE = System.getProperty("com.google.appengine.runtime.version") != null; // For Java6, android and JVM that has no Unsafe class, use Universal MessageBuffer (based on ByteBuffer). - // Java9 onward doesn't allow to access Cleaner by reflection, so we use Universal MessageBuffer again. useUniversalBuffer = Boolean.parseBoolean(System.getProperty("msgpack.universal-buffer", "false")) || isAndroid || isGAE || javaVersion < 7 - || javaVersion > 9 || !hasUnsafe; if (!useUniversalBuffer) { From f3b14bbb4cdda688c4e360fb05af6a1d66facf7f Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sat, 9 Nov 2019 00:23:20 +0100 Subject: [PATCH 4/9] Run Java 11 tests on universal buffer only Java 11 tests without the "msgpack.universal-buffer" property set where using the universal buffer anyway: Java 11's "java.specification.version" does not contain a dot, so MessageBuffer misidentified it as Java less than 7 and switched to MessageBufferU. --- .travis.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index d1c82a096..02b433d6a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,5 +30,4 @@ matrix: - env: PROJECT=java11 jdk: openjdk11 script: - - ./sbt test - ./sbt test -J-Dmsgpack.universal-buffer=true From 55d8dd3e753adaf817829e67aae0f9b5becba10d Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 10 Nov 2019 14:20:47 +0100 Subject: [PATCH 5/9] Fixes DirectBufferAccess#clean on Java 11 For Java 9+ we switch from a DirectByteBuffer.cleaner().clean() call to Unsafe.invokeCleaner(buffer). --- .travis.yml | 1 + .../core/buffer/DirectBufferAccess.java | 147 ++++++++++++++++-- .../msgpack/core/buffer/MessageBuffer.java | 6 +- 3 files changed, 140 insertions(+), 14 deletions(-) diff --git a/.travis.yml b/.travis.yml index 02b433d6a..d1c82a096 100644 --- a/.travis.yml +++ b/.travis.yml @@ -30,4 +30,5 @@ matrix: - env: PROJECT=java11 jdk: openjdk11 script: + - ./sbt test - ./sbt test -J-Dmsgpack.universal-buffer=true 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 ab86061d3..e5824d04e 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 @@ -19,6 +19,12 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.security.AccessController; +import java.security.PrivilegedAction; + +import org.msgpack.core.MessagePack; + +import sun.misc.Unsafe; /** * Wraps the difference of access methods to DirectBuffers between Android and others. @@ -37,20 +43,24 @@ enum DirectBufferConstructorType } static Method mGetAddress; + // For Java <=8, gets a sun.misc.Cleaner static Method mCleaner; static Method mClean; + // For Java >=9, invokes a jdk.internal.ref.Cleaner + static Method mInvokeCleaner; // TODO We should use MethodHandle for efficiency, but it is not available in JDK6 - static Constructor byteBufferConstructor; + static Constructor byteBufferConstructor; static Class directByteBufferClass; static DirectBufferConstructorType directBufferConstructorType; static Method memoryBlockWrapFromJni; static { try { + final ByteBuffer direct = ByteBuffer.allocateDirect(1); // Find the hidden constructor for DirectByteBuffer - directByteBufferClass = ClassLoader.getSystemClassLoader().loadClass("java.nio.DirectByteBuffer"); - Constructor directByteBufferConstructor = null; + directByteBufferClass = direct.getClass(); + Constructor directByteBufferConstructor = null; DirectBufferConstructorType constructorType = null; Method mbWrap = null; try { @@ -92,17 +102,130 @@ enum DirectBufferConstructorType mGetAddress = directByteBufferClass.getDeclaredMethod("address"); mGetAddress.setAccessible(true); - mCleaner = directByteBufferClass.getDeclaredMethod("cleaner"); - mCleaner.setAccessible(true); - - mClean = mCleaner.getReturnType().getDeclaredMethod("clean"); - mClean.setAccessible(true); + if (MessageBuffer.javaVersion >= 8) { + setupCleanerJava9(direct); + } else { + setupCleanerJava6(direct); + } } catch (Exception e) { throw new RuntimeException(e); } } + private static void setupCleanerJava6(final ByteBuffer direct) { + Object obj; + obj = AccessController.doPrivileged(new PrivilegedAction() { + + @Override + public Object run() { + return getCleanerMethod(direct); + } + }); + if (obj instanceof Throwable) { + throw new RuntimeException((Throwable) obj); + } + mCleaner = (Method) obj; + mCleaner.setAccessible(true); + + obj = AccessController.doPrivileged(new PrivilegedAction() { + + @Override + public Object run() { + return getCleanMethod(direct, mCleaner); + } + }); + if (obj instanceof Throwable) { + throw new RuntimeException((Throwable) obj); + } + mClean = (Method) obj; + mClean.setAccessible(true); + } + + private static void setupCleanerJava9(final ByteBuffer direct) { + Object obj = AccessController.doPrivileged(new PrivilegedAction() { + + @Override + public Object run() { + return getInvokeCleanerMethod(direct); + } + }); + if (obj instanceof Throwable) { + throw new RuntimeException((Throwable) obj); + } + mInvokeCleaner = (Method) obj; + } + + /** + * Checks if we have a usable {@link DirectByteBuffer#cleaner}. + * @param direct a direct buffer + * @return the method or an error + */ + private static Object getCleanerMethod(ByteBuffer direct) + { + try { + Method m = direct.getClass().getDeclaredMethod("cleaner"); + m.invoke(direct); + return m; + } + catch (NoSuchMethodException e) { + return e; + } + catch (InvocationTargetException e) { + return e; + } + catch (IllegalAccessException e) { + return e; + } + } + + /** + * 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 + */ + private static Object getCleanMethod(ByteBuffer direct, Method mCleaner) + { + try { + Method m = mCleaner.getReturnType().getDeclaredMethod("clean"); + Object c = mCleaner.invoke(direct); + m.invoke(c); + return m; + } + catch (NoSuchMethodException e) { + return e; + } + catch (InvocationTargetException e) { + return e; + } + catch (IllegalAccessException e) { + return e; + } + } + + /** + * Checks if we have a usable {@link Unsafe#invokeCleaner}. + * @param direct a direct buffer + * @return the method or an error + */ + 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); + m.invoke(MessageBuffer.unsafe, direct); + return m; + } catch (NoSuchMethodException e) { + return e; + } catch (InvocationTargetException e) { + return e; + } catch (IllegalAccessException e) { + return e; + } + } + static long getAddress(Object base) { try { @@ -119,8 +242,12 @@ static long getAddress(Object base) static void clean(Object base) { try { - Object cleaner = mCleaner.invoke(base); - mClean.invoke(cleaner); + if (MessageBuffer.javaVersion <= 8) { + Object cleaner = mCleaner.invoke(base); + mClean.invoke(cleaner); + } else { + mInvokeCleaner.invoke(MessageBuffer.unsafe, base); + } } catch (Throwable e) { throw new RuntimeException(e); diff --git a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java index 8b7f8d2f4..8628ae785 100644 --- a/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java +++ b/msgpack-core/src/main/java/org/msgpack/core/buffer/MessageBuffer.java @@ -47,6 +47,7 @@ public class MessageBuffer { static final boolean isUniversalBuffer; static final Unsafe unsafe; + static final int javaVersion = getJavaVersion(); /** * Reference to MessageBuffer Constructors @@ -69,9 +70,6 @@ public class MessageBuffer int arrayByteBaseOffset = 16; try { - // Check java version - int javaVersion = getJavaVersion(); - boolean hasUnsafe = false; try { hasUnsafe = Class.forName("sun.misc.Unsafe") != null; @@ -163,7 +161,7 @@ public class MessageBuffer } } - static int getJavaVersion() + private static int getJavaVersion() { String javaVersion = System.getProperty("java.specification.version", ""); int dotPos = javaVersion.indexOf('.'); From 293d928fda43c1ed52e2bfbce4cb5ebd8e0e64db Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 10 Nov 2019 14:33:51 +0100 Subject: [PATCH 6/9] Corrects style --- .../core/buffer/DirectBufferAccess.java | 43 +++++++++++-------- 1 file changed, 26 insertions(+), 17 deletions(-) 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 e5824d04e..1c199a714 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 @@ -22,8 +22,6 @@ import java.security.AccessController; import java.security.PrivilegedAction; -import org.msgpack.core.MessagePack; - import sun.misc.Unsafe; /** @@ -104,7 +102,8 @@ enum DirectBufferConstructorType if (MessageBuffer.javaVersion >= 8) { setupCleanerJava9(direct); - } else { + } + else { setupCleanerJava6(direct); } } @@ -113,12 +112,15 @@ enum DirectBufferConstructorType } } - private static void setupCleanerJava6(final ByteBuffer direct) { + private static void setupCleanerJava6(final ByteBuffer direct) + { Object obj; - obj = AccessController.doPrivileged(new PrivilegedAction() { + obj = AccessController.doPrivileged(new PrivilegedAction() + { @Override - public Object run() { + public Object run() + { return getCleanerMethod(direct); } }); @@ -128,10 +130,11 @@ public Object run() { mCleaner = (Method) obj; mCleaner.setAccessible(true); - obj = AccessController.doPrivileged(new PrivilegedAction() { - + obj = AccessController.doPrivileged(new PrivilegedAction() + { @Override - public Object run() { + public Object run() + { return getCleanMethod(direct, mCleaner); } }); @@ -142,11 +145,13 @@ public Object run() { mClean.setAccessible(true); } - private static void setupCleanerJava9(final ByteBuffer direct) { - Object obj = AccessController.doPrivileged(new PrivilegedAction() { - + private static void setupCleanerJava9(final ByteBuffer direct) + { + Object obj = AccessController.doPrivileged(new PrivilegedAction() + { @Override - public Object run() { + public Object run() + { return getInvokeCleanerMethod(direct); } }); @@ -217,11 +222,14 @@ private static Object getInvokeCleanerMethod(ByteBuffer direct) "invokeCleaner", ByteBuffer.class); m.invoke(MessageBuffer.unsafe, direct); return m; - } catch (NoSuchMethodException e) { + } + catch (NoSuchMethodException e) { return e; - } catch (InvocationTargetException e) { + } + catch (InvocationTargetException e) { return e; - } catch (IllegalAccessException e) { + } + catch (IllegalAccessException e) { return e; } } @@ -245,7 +253,8 @@ static void clean(Object base) if (MessageBuffer.javaVersion <= 8) { Object cleaner = mCleaner.invoke(base); mClean.invoke(cleaner); - } else { + } + else { mInvokeCleaner.invoke(MessageBuffer.unsafe, base); } } From 42ef57556f1d4fe1af63659a258c7029e06b9da1 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 10 Nov 2019 14:38:50 +0100 Subject: [PATCH 7/9] Corrects whitespace --- .../main/java/org/msgpack/core/buffer/DirectBufferAccess.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 1c199a714..98cfc4770 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 @@ -117,7 +117,6 @@ private static void setupCleanerJava6(final ByteBuffer direct) Object obj; obj = AccessController.doPrivileged(new PrivilegedAction() { - @Override public Object run() { @@ -254,7 +253,7 @@ static void clean(Object base) Object cleaner = mCleaner.invoke(base); mClean.invoke(cleaner); } - else { + else { mInvokeCleaner.invoke(MessageBuffer.unsafe, base); } } From bd2e6f9094084468596e6ca3c72a22e315d4814a Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 10 Nov 2019 14:48:06 +0100 Subject: [PATCH 8/9] Restores Java8 tests. --- .../main/java/org/msgpack/core/buffer/DirectBufferAccess.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 98cfc4770..b04183591 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 @@ -100,7 +100,7 @@ enum DirectBufferConstructorType mGetAddress = directByteBufferClass.getDeclaredMethod("address"); mGetAddress.setAccessible(true); - if (MessageBuffer.javaVersion >= 8) { + if (MessageBuffer.javaVersion >= 9) { setupCleanerJava9(direct); } else { From 2860e9c6b55f5d5232e5d2cac9b57ade7af67f4d Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 10 Nov 2019 15:46:38 +0100 Subject: [PATCH 9/9] Fixes IllegalAccessExceptions Adds missing setAccessible calls. --- .../org/msgpack/core/buffer/DirectBufferAccess.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) 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 b04183591..cde2e6eca 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 @@ -100,11 +100,11 @@ enum DirectBufferConstructorType mGetAddress = directByteBufferClass.getDeclaredMethod("address"); mGetAddress.setAccessible(true); - if (MessageBuffer.javaVersion >= 9) { - setupCleanerJava9(direct); + if (MessageBuffer.javaVersion <= 8) { + setupCleanerJava6(direct); } else { - setupCleanerJava6(direct); + setupCleanerJava9(direct); } } catch (Exception e) { @@ -127,7 +127,6 @@ public Object run() throw new RuntimeException((Throwable) obj); } mCleaner = (Method) obj; - mCleaner.setAccessible(true); obj = AccessController.doPrivileged(new PrivilegedAction() { @@ -141,7 +140,6 @@ public Object run() throw new RuntimeException((Throwable) obj); } mClean = (Method) obj; - mClean.setAccessible(true); } private static void setupCleanerJava9(final ByteBuffer direct) @@ -169,6 +167,7 @@ private static Object getCleanerMethod(ByteBuffer direct) { try { Method m = direct.getClass().getDeclaredMethod("cleaner"); + m.setAccessible(true); m.invoke(direct); return m; } @@ -194,6 +193,7 @@ private static Object getCleanMethod(ByteBuffer direct, Method mCleaner) try { Method m = mCleaner.getReturnType().getDeclaredMethod("clean"); Object c = mCleaner.invoke(direct); + m.setAccessible(true); m.invoke(c); return m; }