From 0d71744d5b813cbd72e7a7cbc78b27add4f87938 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Mon, 9 Nov 2015 11:45:37 -0800 Subject: [PATCH] IllegalRefCountException in FullHttp[Request|Response].hashCode() Motivation: FullHttp[Request|Response].hashCode() uses a releasable object and in vulnerable to a IllegalRefCountException if that object has been released. Modifications: - Ensure the released object is not used. Result: No more IllegalRefCountException. --- .../codec/http/DefaultFullHttpRequest.java | 28 ++++++++++--- .../codec/http/DefaultFullHttpResponse.java | 42 ++++++++++++++++++- 2 files changed, 63 insertions(+), 7 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java index 87b2cf5e12..2ca9329e7c 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpRequest.java @@ -17,15 +17,19 @@ package io.netty.handler.codec.http; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.util.IllegalReferenceCountException; /** * Default implementation of {@link FullHttpRequest}. */ public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHttpRequest { - private static final int HASH_CODE_PRIME = 31; private final ByteBuf content; private final HttpHeaders trailingHeader; private final boolean validateHeaders; + /** + * Used to cache the value of the hash code and avoid {@link IllegalRefCountException}. + */ + private int hash; public DefaultFullHttpRequest(HttpVersion httpVersion, HttpMethod method, String uri) { this(httpVersion, method, uri, Unpooled.buffer(0)); @@ -163,11 +167,23 @@ public class DefaultFullHttpRequest extends DefaultHttpRequest implements FullHt @Override public int hashCode() { - int result = 1; - result = HASH_CODE_PRIME * result + content().hashCode(); - result = HASH_CODE_PRIME * result + trailingHeaders().hashCode(); - result = HASH_CODE_PRIME * result + super.hashCode(); - return result; + int hash = this.hash; + if (hash == 0) { + if (content().refCnt() != 0) { + try { + hash = 31 + content().hashCode(); + } catch (IllegalReferenceCountException ignored) { + // Handle race condition between checking refCnt() == 0 and using the object. + hash = 31; + } + } else { + hash = 31; + } + hash = 31 * hash + trailingHeaders().hashCode(); + hash = 31 * hash + super.hashCode(); + this.hash = hash; + } + return hash; } @Override diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java index 36ac176d64..5ad6058daf 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/DefaultFullHttpResponse.java @@ -15,9 +15,11 @@ */ package io.netty.handler.codec.http; -import static io.netty.util.internal.ObjectUtil.checkNotNull; import io.netty.buffer.ByteBuf; import io.netty.buffer.Unpooled; +import io.netty.util.IllegalReferenceCountException; + +import static io.netty.util.internal.ObjectUtil.checkNotNull; /** * Default implementation of a {@link FullHttpResponse}. @@ -27,6 +29,10 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full private final ByteBuf content; private final HttpHeaders trailingHeaders; private final boolean validateHeaders; + /** + * Used to cache the value of the hash code and avoid {@link IllegalRefCountException}. + */ + private int hash; public DefaultFullHttpResponse(HttpVersion version, HttpResponseStatus status) { this(version, status, Unpooled.buffer(0)); @@ -164,6 +170,40 @@ public class DefaultFullHttpResponse extends DefaultHttpResponse implements Full return duplicate; } + @Override + public int hashCode() { + int hash = this.hash; + if (hash == 0) { + if (content().refCnt() != 0) { + try { + hash = 31 + content().hashCode(); + } catch (IllegalReferenceCountException ignored) { + // Handle race condition between checking refCnt() == 0 and using the object. + hash = 31; + } + } else { + hash = 31; + } + hash = 31 * hash + trailingHeaders().hashCode(); + hash = 31 * hash + super.hashCode(); + this.hash = hash; + } + return hash; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof DefaultFullHttpResponse)) { + return false; + } + + DefaultFullHttpResponse other = (DefaultFullHttpResponse) o; + + return super.equals(other) && + content().equals(other.content()) && + trailingHeaders().equals(other.trailingHeaders()); + } + @Override public String toString() { return HttpMessageUtil.appendFullResponse(new StringBuilder(256), this).toString();