From 38eee409c8ca356d4a0254919e5898ecd9a2100c Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Thu, 30 Aug 2018 07:43:10 +0200 Subject: [PATCH] =?UTF-8?q?We=20should=20be=20able=20to=20use=20the=20Byte?= =?UTF-8?q?Buffer=20cleaner=20on=20java8=20(and=20earlier=E2=80=A6=20(#823?= =?UTF-8?q?4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * We should be able to use the ByteBuffer cleaner on java8 (and earlier versions) even if sun.misc.Unsafe is not present. Motivation: At the moment we have a hard dependency on sun.misc.Unsafe to use the Cleaner on Java8 and earlier. This is not really needed as we can still use pure reflection if sun.misc.Unsafe is not present. Modifications: Refactor Cleaner6 to fallback to pure reflection if sun.misc.Unsafe is not present on system. Result: More timely releasing of direct memory on Java8 and earlier when sun.misc.Unsafe is not present. --- .../io/netty/util/internal/CleanerJava6.java | 111 ++++++++++++++---- .../util/internal/PlatformDependent.java | 2 +- 2 files changed, 89 insertions(+), 24 deletions(-) diff --git a/common/src/main/java/io/netty/util/internal/CleanerJava6.java b/common/src/main/java/io/netty/util/internal/CleanerJava6.java index 8bbdbf7240..3835823603 100644 --- a/common/src/main/java/io/netty/util/internal/CleanerJava6.java +++ b/common/src/main/java/io/netty/util/internal/CleanerJava6.java @@ -21,6 +21,8 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import java.lang.reflect.Field; import java.lang.reflect.Method; import java.nio.ByteBuffer; +import java.security.AccessController; +import java.security.PrivilegedAction; /** @@ -32,41 +34,72 @@ import java.nio.ByteBuffer; final class CleanerJava6 implements Cleaner { private static final long CLEANER_FIELD_OFFSET; private static final Method CLEAN_METHOD; + private static final Field CLEANER_FIELD; private static final InternalLogger logger = InternalLoggerFactory.getInstance(CleanerJava6.class); static { - long fieldOffset = -1; - Method clean = null; + long fieldOffset; + Method clean; + Field cleanerField; Throwable error = null; - if (PlatformDependent0.hasUnsafe()) { - ByteBuffer direct = ByteBuffer.allocateDirect(1); - try { - Field cleanerField = direct.getClass().getDeclaredField("cleaner"); - fieldOffset = PlatformDependent0.objectFieldOffset(cleanerField); - Object cleaner = PlatformDependent0.getObject(direct, fieldOffset); - clean = cleaner.getClass().getDeclaredMethod("clean"); - clean.invoke(cleaner); - } catch (Throwable t) { - // We don't have ByteBuffer.cleaner(). - fieldOffset = -1; - clean = null; - error = t; + final ByteBuffer direct = ByteBuffer.allocateDirect(1); + try { + Object mayBeCleanerField = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Object run() { + try { + Field cleanerField = direct.getClass().getDeclaredField("cleaner"); + if (!PlatformDependent.hasUnsafe()) { + // We need to make it accessible if we do not use Unsafe as we will access it via + // reflection. + cleanerField.setAccessible(true); + } + return cleanerField; + } catch (Throwable cause) { + return cause; + } + } + }); + if (mayBeCleanerField instanceof Throwable) { + throw (Throwable) mayBeCleanerField; } - } else { - error = new UnsupportedOperationException("sun.misc.Unsafe unavailable"); + + cleanerField = (Field) mayBeCleanerField; + + final Object cleaner; + + // If we have sun.misc.Unsafe we will use it as its faster then using reflection, + // otherwise let us try reflection as last resort. + if (PlatformDependent.hasUnsafe()) { + fieldOffset = PlatformDependent0.objectFieldOffset(cleanerField); + cleaner = PlatformDependent0.getObject(direct, fieldOffset); + } else { + fieldOffset = -1; + cleaner = cleanerField.get(direct); + } + clean = cleaner.getClass().getDeclaredMethod("clean"); + clean.invoke(cleaner); + } catch (Throwable t) { + // We don't have ByteBuffer.cleaner(). + fieldOffset = -1; + clean = null; + error = t; + cleanerField = null; } + if (error == null) { logger.debug("java.nio.ByteBuffer.cleaner(): available"); } else { logger.debug("java.nio.ByteBuffer.cleaner(): unavailable", error); } + CLEANER_FIELD = cleanerField; CLEANER_FIELD_OFFSET = fieldOffset; CLEAN_METHOD = clean; } static boolean isSupported() { - return CLEANER_FIELD_OFFSET != -1; + return CLEANER_FIELD_OFFSET != -1 || CLEANER_FIELD != null; } @Override @@ -74,13 +107,45 @@ final class CleanerJava6 implements Cleaner { if (!buffer.isDirect()) { return; } - try { - Object cleaner = PlatformDependent0.getObject(buffer, CLEANER_FIELD_OFFSET); - if (cleaner != null) { - CLEAN_METHOD.invoke(cleaner); + if (System.getSecurityManager() == null) { + try { + freeDirectBuffer0(buffer); + } catch (Throwable cause) { + PlatformDependent0.throwException(cause); } - } catch (Throwable cause) { + } else { + freeDirectBufferPrivileged(buffer); + } + } + + private static void freeDirectBufferPrivileged(final ByteBuffer buffer) { + Throwable cause = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public Throwable run() { + try { + freeDirectBuffer0(buffer); + return null; + } catch (Throwable cause) { + return cause; + } + } + }); + if (cause != null) { PlatformDependent0.throwException(cause); } } + + private static void freeDirectBuffer0(ByteBuffer buffer) throws Exception { + final Object cleaner; + // If CLEANER_FIELD_OFFSET == -1 we need to use reflection to access the cleaner, otherwise we can use + // sun.misc.Unsafe. + if (CLEANER_FIELD_OFFSET == -1) { + cleaner = CLEANER_FIELD.get(buffer); + } else { + cleaner = PlatformDependent0.getObject(buffer, CLEANER_FIELD_OFFSET); + } + if (cleaner != null) { + CLEAN_METHOD.invoke(cleaner); + } + } } 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 8d44920cc2..64d740aaf3 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent.java @@ -164,7 +164,7 @@ public final class PlatformDependent { MAYBE_SUPER_USER = maybeSuperUser0(); - if (!isAndroid() && hasUnsafe()) { + if (!isAndroid()) { // only direct to method if we are not running on android. // See https://github.com/netty/netty/issues/2604 if (javaVersion() >= 9) {