From 57f90ae7d19332d9d919d4cbd2123a27b55ab2cd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Thu, 20 Nov 2014 13:00:02 +0100 Subject: [PATCH] 452465 - 100% CPU spin on page reload. Fixed by adding a MetaData.recycle() method that properly recycles the MetaData.Request object so that HttpChannelOverHttp.earlyEOF() properly closes the connection when it's idle. --- .../java/org/eclipse/jetty/http/MetaData.java | 254 ++++++++---------- .../jetty/server/HttpChannelOverHttp.java | 3 +- 2 files changed, 109 insertions(+), 148 deletions(-) diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java index 68ac39ab516..e3ebed6e248 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/MetaData.java @@ -18,109 +18,119 @@ package org.eclipse.jetty.http; +import java.util.Collections; import java.util.Iterator; +import java.util.Objects; public class MetaData implements Iterable { private HttpVersion _httpVersion; private HttpFields _fields; - long _contentLength=Long.MIN_VALUE; + private long _contentLength; - /* ------------------------------------------------------------ */ public MetaData() { + this(null, null); } - - /* ------------------------------------------------------------ */ + public MetaData(HttpVersion version, HttpFields fields) { - _httpVersion = version; - _fields = fields; + this(version, fields, Long.MIN_VALUE); } - - /* ------------------------------------------------------------ */ + public MetaData(HttpVersion version, HttpFields fields, long contentLength) { _httpVersion = version; _fields = fields; - _contentLength=contentLength; + _contentLength = contentLength; + } + + protected void recycle() + { + _httpVersion = null; + if (_fields != null) + _fields.clear(); + _contentLength = Long.MIN_VALUE; } - /* ------------------------------------------------------------ */ public boolean isRequest() { return false; } - /* ------------------------------------------------------------ */ public boolean isResponse() { return false; } - /* ------------------------------------------------------------ */ - /** Get the httpVersion. - * @return the httpVersion + /** + * @return the HTTP version of this MetaData object */ public HttpVersion getVersion() { return _httpVersion; } - /* ------------------------------------------------------------ */ - /** Set the httpVersion. - * @param httpVersion the httpVersion to set + /** + * @param httpVersion the HTTP version to set */ public void setHttpVersion(HttpVersion httpVersion) { _httpVersion = httpVersion; } - /* ------------------------------------------------------------ */ - /** Get the fields. - * @return the fields + /** + * @return the HTTP fields of this MetaData object */ public HttpFields getFields() { return _fields; } - /* ------------------------------------------------------------ */ - /** Set the fields. - * @param fields the fields to set + /** + * @param fields the HTTP fields to set */ public void setFields(HttpFields fields) { _fields = fields; - _contentLength=Long.MIN_VALUE; + _contentLength = Long.MIN_VALUE; } - /* ------------------------------------------------------------ */ + /** + * @return the content length if available, otherwise {@link Long#MIN_VALUE} + */ public long getContentLength() { - if (_contentLength==Long.MIN_VALUE) + if (_contentLength == Long.MIN_VALUE) { - HttpField cl = _fields.getField(HttpHeader.CONTENT_LENGTH); - _contentLength=(cl==null)?-1:cl.getLongValue(); + if (_fields != null) + { + HttpField field = _fields.getField(HttpHeader.CONTENT_LENGTH); + _contentLength = field == null ? -1 : field.getLongValue(); + } } - return _contentLength; } - - /* ------------------------------------------------------------ */ + + /** + * @return an iterator over the HTTP fields + * @see #getFields() + */ public Iterator iterator() { - return getFields().iterator(); + HttpFields fields = getFields(); + return fields == null ? Collections.emptyIterator() : fields.iterator(); } - /* ------------------------------------------------------------ */ @Override public int hashCode() { - return 31 * getVersion().hashCode() + getFields().hashCode(); + HttpVersion version = getVersion(); + int hash = version == null ? 0 : version.hashCode(); + HttpFields fields = getFields(); + return 31 * hash + (fields == null ? 0 : fields.hashCode()); } - /* ------------------------------------------------------------ */ @Override public boolean equals(Object o) { @@ -128,140 +138,122 @@ public class MetaData implements Iterable return true; if (!(o instanceof MetaData)) return false; + MetaData that = (MetaData)o; if (getVersion() != that.getVersion()) return false; - return getFields().equals(that.getFields()); + return Objects.equals(getFields(), that.getFields()); } - /* ------------------------------------------------------------ */ @Override public String toString() { StringBuilder out = new StringBuilder(); - for (HttpField field: this) + for (HttpField field : this) out.append(field).append(System.lineSeparator()); return out.toString(); } - - /* -------------------------------------------------------- */ - /* -------------------------------------------------------- */ - /* -------------------------------------------------------- */ + public static class Request extends MetaData { private String _method; private HttpURI _uri; public Request() - { + { } - /* ------------------------------------------------------------ */ - /** - * @param method - * @param uri - * @param version - * @param fields - */ public Request(String method, HttpURI uri, HttpVersion version, HttpFields fields) { - this(method,uri,version,fields,Long.MIN_VALUE); + this(method, uri, version, fields, Long.MIN_VALUE); } - - /* ------------------------------------------------------------ */ - /** - * @param method - * @param uri - * @param version - * @param fields - */ + public Request(String method, HttpURI uri, HttpVersion version, HttpFields fields, long contentLength) { - super(version,fields,contentLength); + super(version, fields, contentLength); _method = method; _uri = uri; } - /* ------------------------------------------------------------ */ public Request(String method, HttpScheme scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields) { - this(method,new HttpURI(scheme==null?null:scheme.asString(),hostPort.getHost(),hostPort.getPort(),uri),version,fields); + this(method, new HttpURI(scheme == null ? null : scheme.asString(), hostPort.getHost(), hostPort.getPort(), uri), version, fields); } - - /* ------------------------------------------------------------ */ + public Request(String method, HttpScheme scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields, long contentLength) { - this(method,new HttpURI(scheme==null?null:scheme.asString(),hostPort.getHost(),hostPort.getPort(),uri),version,fields,contentLength); + this(method, new HttpURI(scheme == null ? null : scheme.asString(), hostPort.getHost(), hostPort.getPort(), uri), version, fields, contentLength); } - /* ------------------------------------------------------------ */ public Request(String method, String scheme, HostPortHttpField hostPort, String uri, HttpVersion version, HttpFields fields, long contentLength) { - this(method,new HttpURI(scheme,hostPort.getHost(),hostPort.getPort(),uri),version,fields,contentLength); + this(method, new HttpURI(scheme, hostPort.getHost(), hostPort.getPort(), uri), version, fields, contentLength); + } + + public void recycle() + { + super.recycle(); + _method = null; + if (_uri != null) + _uri.clear(); } - /* ------------------------------------------------------------ */ @Override public boolean isRequest() { return true; } - /* ------------------------------------------------------------ */ - /** Get the method. - * @return the method + /** + * @return the HTTP method */ public String getMethod() { return _method; } - /* ------------------------------------------------------------ */ - /** Set the method. - * @param method the method to set + /** + * @param method the HTTP method to set */ public void setMethod(String method) { _method = method; } - /* ------------------------------------------------------------ */ - /** Get the uri. - * @return the uri + /** + * @return the HTTP URI */ public HttpURI getURI() { return _uri; } - - /* ------------------------------------------------------------ */ - /** Get the uri. - * @return the uri + + /** + * @return the HTTP URI in string form */ public String getURIString() { - return _uri==null?null:_uri.toString(); + return _uri == null ? null : _uri.toString(); } - /* ------------------------------------------------------------ */ - /** Set the uri. - * @param uri the uri to set + /** + * @param uri the HTTP URI to set */ public void setURI(HttpURI uri) { _uri = uri; } - - /* ------------------------------------------------------------ */ + @Override public int hashCode() { - return ((super.hashCode()*31)+_method.hashCode())*31+_uri.hashCode(); + int hash = super.hashCode(); + hash = hash * 31 + (_method == null ? 0 : _method.hashCode()); + return hash * 31 + (_uri == null ? 0 : _uri.hashCode()); } - /* ------------------------------------------------------------ */ @Override public boolean equals(Object o) { @@ -270,13 +262,12 @@ public class MetaData implements Iterable if (!(o instanceof MetaData.Request)) return false; MetaData.Request that = (MetaData.Request)o; - if (!getMethod().equals(that.getMethod()) || - !getURI().equals(that.getURI())) + if (!Objects.equals(getMethod(), that.getMethod()) || + !Objects.equals(getURI(), that.getURI())) return false; return super.equals(o); } - - /* ------------------------------------------------------------ */ + @Override public String toString() { @@ -285,9 +276,6 @@ public class MetaData implements Iterable } } - /* -------------------------------------------------------- */ - /* -------------------------------------------------------- */ - /* -------------------------------------------------------- */ public static class Response extends MetaData { private int _status; @@ -295,95 +283,71 @@ public class MetaData implements Iterable public Response() { + this(null, 0, null); } - /* ------------------------------------------------------------ */ - /** - * @param version - * @param fields - * @param status - */ public Response(HttpVersion version, int status, HttpFields fields) { - this(version,status,fields,Long.MIN_VALUE); + this(version, status, fields, Long.MIN_VALUE); } - /* ------------------------------------------------------------ */ - /** - * @param version - * @param fields - * @param status - */ - public Response(HttpVersion version, int status, HttpFields fields,long contentLength) + public Response(HttpVersion version, int status, HttpFields fields, long contentLength) { - super(version,fields,contentLength); - _status=status; - } - - /* ------------------------------------------------------------ */ - /** - * @param version - * @param fields - * @param status - */ - public Response(HttpVersion version, int status, String reason, HttpFields fields,long contentLength) - { - super(version,fields,contentLength); - _reason=reason; - _status=status; + super(version, fields, contentLength); + _status = status; + } + + public Response(HttpVersion version, int status, String reason, HttpFields fields, long contentLength) + { + super(version, fields, contentLength); + _reason = reason; + _status = status; } - /* ------------------------------------------------------------ */ @Override public boolean isResponse() { return true; } - - /* ------------------------------------------------------------ */ - /** Get the status. - * @return the status + + /** + * @return the HTTP status */ public int getStatus() { return _status; } - - /* ------------------------------------------------------------ */ - /** Get the reason. - * @return the status + + /** + * @return the HTTP reason */ public String getReason() { return _reason; } - /* ------------------------------------------------------------ */ - /** Set the status. - * @param status the status to set + /** + * @param status the HTTP status to set */ public void setStatus(int status) { _status = status; } - /* ------------------------------------------------------------ */ - /** Set the reason. - * @param reason the reason to set + /** + * @param reason the HTTP reason to set */ public void setReason(String reason) { _reason = reason; } - /* ------------------------------------------------------------ */ @Override public int hashCode() { - return 31 * getStatus() + super.hashCode(); + return 31 * super.hashCode() + getStatus(); } - /* ------------------------------------------------------------ */ @Override public boolean equals(Object o) { @@ -397,12 +361,10 @@ public class MetaData implements Iterable return super.equals(o); } - /* ------------------------------------------------------------ */ @Override public String toString() { - return String.format("%s %d%s%s",getVersion(), getStatus(), System.lineSeparator(), super.toString()); + return String.format("%s %d%s%s", getVersion(), getStatus(), System.lineSeparator(), super.toString()); } - } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java index 00734991334..fe980280c7f 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannelOverHttp.java @@ -20,7 +20,6 @@ package org.eclipse.jetty.server; import java.io.IOException; -import java.net.InetSocketAddress; import java.nio.ByteBuffer; import org.eclipse.jetty.http.HostPortHttpField; @@ -71,7 +70,7 @@ class HttpChannelOverHttp extends HttpChannel implements HttpParser.RequestHandl _expect = false; _expect100Continue = false; _expect102Processing = false; - _metadata.getURI().clear(); + _metadata.recycle(); _connection=null; _fields.clear(); }