diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java index c095d8a654..bcec34d960 100644 --- a/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java +++ b/codec/src/main/java/io/netty/handler/codec/ByteToMessageCodec.java @@ -89,7 +89,7 @@ public abstract class ByteToMessageCodec extends ChannelDuplexHandler { } private void checkForSharableAnnotation() { - if (getClass().isAnnotationPresent(Sharable.class)) { + if (isSharable()) { throw new IllegalStateException("@Sharable annotation is not allowed"); } } diff --git a/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java b/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java index a086760051..dec65dfa4c 100644 --- a/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java +++ b/codec/src/main/java/io/netty/handler/codec/ByteToMessageDecoder.java @@ -52,7 +52,7 @@ public abstract class ByteToMessageDecoder extends ChannelInboundHandlerAdapter private boolean first; protected ByteToMessageDecoder() { - if (getClass().isAnnotationPresent(Sharable.class)) { + if (isSharable()) { throw new IllegalStateException("@Sharable annotation is not allowed"); } } diff --git a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java index 8d315025ba..3cb1cc29f5 100644 --- a/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java +++ b/transport/src/main/java/io/netty/channel/ChannelHandlerAdapter.java @@ -16,11 +16,31 @@ package io.netty.channel; +import java.util.Map; +import java.util.WeakHashMap; + /** * Skelton implementation of a {@link ChannelHandler}. */ public abstract class ChannelHandlerAdapter implements ChannelHandler { + /** + * Cache the result of {@link Sharable} annotation detection to workaround a condition. We use a + * {@link ThreadLocal} and {@link WeakHashMap} to eliminate the volatile write/reads. Using different + * {@link WeakHashMap} instances per {@link Thread} is good enough for us and the number of + * {@link Thread}s are quite limited anyway. + * + * See #2289. + */ + private static final ThreadLocal, Boolean>> SHARABLE_CACHE = + new ThreadLocal, Boolean>>() { + @Override + protected Map, Boolean> initialValue() { + // Start with small capacity to keep memory overhead as low as possible. + return new WeakHashMap, Boolean>(4); + } + }; + // Not using volatile because it's used only for a sanity check. boolean added; @@ -29,7 +49,14 @@ public abstract class ChannelHandlerAdapter implements ChannelHandler { * to different {@link ChannelPipeline}s. */ public boolean isSharable() { - return getClass().isAnnotationPresent(Sharable.class); + Class clazz = getClass(); + Map, Boolean> cache = SHARABLE_CACHE.get(); + Boolean sharable = cache.get(clazz); + if (sharable == null) { + sharable = clazz.isAnnotationPresent(Sharable.class); + cache.put(clazz, sharable); + } + return sharable; } /**