From ab72dd73036292580e9be75c0286c3b6b34888ea Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 20 Mar 2014 11:11:07 +0900 Subject: [PATCH] Fix and simplify freeing a direct buffer / Fix Android support Motivation: 6e8ba291cfcb325fe4397a8e7db43579070e7a07 introduced a regression in Android because Android does not have sun.nio.ch.DirectBuffer (see #2330.) I also found PlatformDependent0.freeDirectBuffer() and freeDirectBufferUnsafe() are pretty much same after the commit and the unsafe version should be removed. Modifications: - Do not use the pooled allocator in Android because it's too resource hungry for Androids. - Merge PlatformDependent0.freeDirectBuffer() and freeDirectBufferUnsafe() into one method. - Make the Unsafe unavailable when sun.nio.ch.DirectBuffer is unavailable. We could keep the Unsafe available and handle the sun.nio.ch.DirectBuffer case separately, but I don't want to complicate our code just because of that. All supported JDK versions have sun.nio.ch.DirectBuffer if the Unsafe is available. Result: Simpler code. Fixes Android support (#2330) --- .../java/io/netty/buffer/ByteBufUtil.java | 6 ++- .../util/internal/PlatformDependent.java | 8 +--- .../util/internal/PlatformDependent0.java | 40 +++++++------------ 3 files changed, 22 insertions(+), 32 deletions(-) diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java index 23f3bd5182..0aa9807f1b 100644 --- a/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java +++ b/buffer/src/main/java/io/netty/buffer/ByteBufUtil.java @@ -16,6 +16,7 @@ package io.netty.buffer; import io.netty.util.CharsetUtil; +import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.SystemPropertyUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -48,7 +49,10 @@ public final class ByteBufUtil { HEXDUMP_TABLE[(i << 1) + 1] = DIGITS[i & 0x0F]; } - String allocType = SystemPropertyUtil.get("io.netty.allocator.type", "pooled").toLowerCase(Locale.US).trim(); + String allocType = SystemPropertyUtil.get( + "io.netty.allocator.type", PlatformDependent.isAndroid() ? "unpooled" : "pooled"); + allocType = allocType.toLowerCase(Locale.US).trim(); + ByteBufAllocator alloc; if ("unpooled".equals(allocType)) { alloc = UnpooledByteBufAllocator.DEFAULT; diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent.java b/common/src/main/java/io/netty/util/internal/PlatformDependent.java index f26fea64b6..16cfc7c51c 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent.java @@ -251,12 +251,8 @@ public final class PlatformDependent { * the current platform does not support this operation or the specified buffer is not a direct buffer. */ public static void freeDirectBuffer(ByteBuffer buffer) { - if (buffer.isDirect()) { - if (hasUnsafe()) { - PlatformDependent0.freeDirectBufferUnsafe(buffer); - } else { - PlatformDependent0.freeDirectBuffer(buffer); - } + if (hasUnsafe()) { + PlatformDependent0.freeDirectBuffer(buffer); } } diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java index d38f196e33..048ef4d6e0 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent0.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent0.java @@ -48,39 +48,40 @@ final class PlatformDependent0 { private static final boolean UNALIGNED; static { - ByteBuffer direct = ByteBuffer.allocateDirect(1); - Field cleanerField; + boolean directBufferFreeable = false; try { - cleanerField = direct.getClass().getDeclaredField("cleaner"); - cleanerField.setAccessible(true); - Cleaner cleaner = (Cleaner) cleanerField.get(direct); - cleaner.clean(); + Class cls = Class.forName("sun.nio.ch.DirectBuffer", false, PlatformDependent0.class.getClassLoader()); + Method method = cls.getMethod("cleaner"); + if ("sun.misc.Cleaner".equals(method.getReturnType().getName())) { + directBufferFreeable = true; + } } catch (Throwable t) { - cleanerField = null; + // We don't have sun.nio.ch.DirectBuffer.cleaner(). } - logger.debug("java.nio.ByteBuffer.cleaner: {}", cleanerField != null? "available" : "unavailable"); + logger.debug("sun.nio.ch.DirectBuffer.cleaner(): {}", directBufferFreeable? "available" : "unavailable"); Field addressField; try { addressField = Buffer.class.getDeclaredField("address"); addressField.setAccessible(true); if (addressField.getLong(ByteBuffer.allocate(1)) != 0) { + // A heap buffer must have 0 address. addressField = null; } else { - direct = ByteBuffer.allocateDirect(1); + ByteBuffer direct = ByteBuffer.allocateDirect(1); if (addressField.getLong(direct) == 0) { + // A direct buffer must have non-zero address. addressField = null; } - Cleaner cleaner = (Cleaner) cleanerField.get(direct); - cleaner.clean(); } } catch (Throwable t) { + // Failed to access the address field. addressField = null; } logger.debug("java.nio.Buffer.address: {}", addressField != null? "available" : "unavailable"); Unsafe unsafe; - if (addressField != null && cleanerField != null) { + if (addressField != null && directBufferFreeable) { try { Field unsafeField = Unsafe.class.getDeclaredField("theUnsafe"); unsafeField.setAccessible(true); @@ -104,6 +105,7 @@ final class PlatformDependent0 { throw e; } } catch (Throwable cause) { + // Unsafe.copyMemory(Object, long, Object, long, long) unavailable. unsafe = null; } } else { @@ -111,6 +113,7 @@ final class PlatformDependent0 { // Let's just pretend unsafe is unavailable for overall simplicity. unsafe = null; } + UNSAFE = unsafe; if (unsafe == null) { @@ -144,19 +147,6 @@ final class PlatformDependent0 { UNSAFE.throwException(t); } - static void freeDirectBufferUnsafe(ByteBuffer buffer) { - try { - Cleaner cleaner = ((DirectBuffer) buffer).cleaner(); - if (cleaner == null) { - throw new IllegalArgumentException( - "attempted to deallocate the buffer which was allocated via JNIEnv->NewDirectByteBuffer()"); - } - cleaner.clean(); - } catch (Throwable t) { - // Nothing we can do here. - } - } - static void freeDirectBuffer(ByteBuffer buffer) { if (!(buffer instanceof DirectBuffer)) { return;