From c6094c239854c4c2e37a0bacdaa6abd795356144 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 22 Aug 2012 11:52:50 +0200 Subject: [PATCH] Jetty9 - Small cleanups. --- .../org/eclipse/jetty/server/HttpChannel.java | 87 ++----------------- .../eclipse/jetty/server/HttpConnection.java | 18 ++-- .../org/eclipse/jetty/server/HttpInput.java | 61 +++++++------ .../eclipse/jetty/server/HttpWriterTest.java | 5 -- .../eclipse/jetty/server/ResponseTest.java | 5 -- 5 files changed, 49 insertions(+), 127 deletions(-) diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java index 61198a2d264..42310fdb54d 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpChannel.java @@ -295,89 +295,14 @@ public class HttpChannel implements HttpParser.RequestHandler } catch (IOException x) { - x.printStackTrace(); // TODO + // We cannot write the response, so there is no point in calling + // response.sendError() since that writes, and we already know we cannot write. + LOG.debug("Could not write response", x); } return true; } - // TODO: remove this method - protected void completed() - { - /* - // This method is called by handle() when it knows that its handling of the request/response cycle - // is complete. - // This may happen in the original thread dispatched to the connection that has called handle(), - // or it may be from a thread dispatched to call handle() as the result of a resumed suspended request. - - LOG.debug("{} complete", this); - - - // Handle connection upgrades - if (_response.getStatus() == HttpStatus.SWITCHING_PROTOCOLS_101) - { - Connection connection = (Connection)getRequest().getAttribute(HttpConnection.UPGRADE_CONNECTION_ATTRIBUTE); - if (connection != null) - { - LOG.debug("Upgrade from {} to {}", this, connection); - getEndPoint().setConnection(connection); - // HttpConnection.this.reset(); // TODO: this should be done by the connection privately when handle returns - return; - } - } - - - // Reset everything for the next cycle. - // HttpConnection.this.reset(); // TODO: this should be done by the connection privately when handle returns - - // are called from non connection thread (ie dispatched from a resume) - if (getCurrentConnection()!=HttpConnection.this) - { - if (_parser.isStart()) - { - // it wants to eat more - if (_requestBuffer==null) - fillInterested(); - else if (getConnector().isStarted()) - { - LOG.debug("{} pipelined",this); - - try - { - execute(this); - } - catch(RejectedExecutionException e) - { - if (getConnector().isStarted()) - LOG.warn(e); - else - LOG.ignore(e); - getEndPoint().close(); - } - } - else - getEndPoint().close(); - } - - if (_parser.isClosed()&&!getEndPoint().isOutputShutdown()) - { - // TODO This is a catch all indicating some protocol handling failure - // Currently needed for requests saying they are HTTP/2.0. - // This should be removed once better error handling is in place - LOG.warn("Endpoint output not shutdown when seeking EOF"); - getEndPoint().shutdownOutput(); - } - } - - // make sure that an oshut connection is driven towards close - // TODO this is a little ugly - if (getEndPoint().isOpen() && getEndPoint().isOutputShutdown()) - { - fillInterested(); - } - */ - } - /** *

Sends an error 500, performing a special logic to detect whether the request is suspended, * to avoid concurrent writes from the application.

@@ -485,7 +410,7 @@ public class HttpChannel implements HttpParser.RequestHandler // TODO: is this needed ? if (_state.isIdle()) _state.complete(); - _request.getHttpInput().shutdownInput(); + _request.getHttpInput().shutdown(); } catch (IOException x) { @@ -694,14 +619,14 @@ public class HttpChannel implements HttpParser.RequestHandler @Override public boolean messageComplete(long contentLength) { - _request.getHttpInput().shutdownInput(); + _request.getHttpInput().shutdown(); return true; } @Override public boolean earlyEOF() { - _request.getHttpInput().shutdownInput(); + _request.getHttpInput().shutdown(); return false; } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java index 1301c22ba91..83ad4b4e9c1 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java @@ -36,7 +36,7 @@ import org.eclipse.jetty.util.log.Logger; /** *

A {@link Connection} that handles the HTTP protocol.

*/ -public class HttpConnection extends AbstractConnection +public class HttpConnection extends AbstractConnection implements Runnable { public static final String UPGRADE_CONNECTION_ATTRIBUTE = "org.eclispe.jetty.server.HttpConnection.UPGRADE"; private static final Logger LOG = Log.getLogger(HttpConnection.class); @@ -228,15 +228,7 @@ public class HttpConnection extends AbstractConnection try { - // TODO: avoid object creation - getExecutor().execute(new Runnable() - { - @Override - public void run() - { - onFillable(); - } - }); + getExecutor().execute(this); } catch (RejectedExecutionException e) { @@ -308,4 +300,10 @@ public class HttpConnection extends AbstractConnection super.onOpen(); fillInterested(); } + + @Override + public void run() + { + onFillable(); + } } diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java index a32853dd2e5..3f43ea99877 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java @@ -21,7 +21,6 @@ package org.eclipse.jetty.server; import java.io.IOException; import java.io.InterruptedIOException; import java.nio.ByteBuffer; - import javax.servlet.ServletInputStream; import org.eclipse.jetty.util.ArrayQueue; @@ -34,22 +33,22 @@ import org.eclipse.jetty.util.log.Logger; /** * This class provides an HttpInput stream for the {@link HttpChannel}. * The input stream holds a queue of {@link ByteBuffer}s passed to it by - * calls to {@link #content(ByteBuffer)}. This class does not copy the buffers, - * but simply holds references to them, thus the caller must organise for those + * calls to {@link #content(ByteBuffer)}. This class does not copy the buffers, + * but simply holds references to them, thus the caller must organise for those * buffers to valid while held by this class. To assist the caller, there are * extensible methods {@link #onContentQueued(ByteBuffer)}, {@link #onContentConsumed(ByteBuffer)} - * and {@link #onAllContentConsumed()} that can be implemented so that the + * and {@link #onAllContentConsumed()} that can be implemented so that the * creator of HttpInput will know when buffers are queued and dequeued. */ public class HttpInput extends ServletInputStream { private static final Logger LOG = Log.getLogger(HttpInput.class); - + protected final byte[] _oneByte=new byte[1]; protected final ArrayQueue _inputQ=new ArrayQueue<>(); private ByteBuffer _content; private boolean _inputEOF; - + /* ------------------------------------------------------------ */ public HttpInput() { @@ -60,14 +59,14 @@ public class HttpInput extends ServletInputStream { return _inputQ.lock(); } - + /* ------------------------------------------------------------ */ public void recycle() { synchronized (lock()) { _inputEOF=false; - + if (_content!=null) onContentConsumed(_content); while ((_content=_inputQ.poll())!=null) @@ -77,7 +76,7 @@ public class HttpInput extends ServletInputStream _content=null; } } - + /* ------------------------------------------------------------ */ /* * @see java.io.InputStream#read() @@ -104,7 +103,7 @@ public class HttpInput extends ServletInputStream } /* ------------------------------------------------------------ */ - /* + /* * @see java.io.InputStream#read(byte[], int, int) */ @Override @@ -112,7 +111,7 @@ public class HttpInput extends ServletInputStream { synchronized (lock()) { - ByteBuffer content=null; + ByteBuffer content=null; while(content==null) { content=_inputQ.peekUnsafe(); @@ -130,17 +129,17 @@ public class HttpInput extends ServletInputStream onEof(); return -1; } - + blockForContent(); } } - + int l=Math.min(len,content.remaining()); content.get(b,off,l); return l; } } - + protected void blockForContent() throws IOException { synchronized (lock()) @@ -158,44 +157,53 @@ public class HttpInput extends ServletInputStream } } } - + protected void onContentQueued(ByteBuffer ref) { lock().notify(); } - + protected void onContentConsumed(ByteBuffer ref) - { + { } - + protected void onAllContentConsumed() - { + { } - + protected void onEof() - { + { } - + public boolean content(ByteBuffer ref) { synchronized (lock()) { _inputQ.add(ref); onContentQueued(ref); - } + } return true; } - public void shutdownInput() + public void shutdown() { synchronized (lock()) - { + { _inputEOF=true; } } - + + public boolean isShutdown() + { + synchronized (lock()) + { + return _inputEOF; + } + } + public void consumeAll() { +/* while (true) { synchronized (lock()) @@ -213,5 +221,6 @@ public class HttpInput extends ServletInputStream LOG.warn(e); } } +*/ } } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpWriterTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpWriterTest.java index 8804cfd265f..6fda7355fc2 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/HttpWriterTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/HttpWriterTest.java @@ -51,11 +51,6 @@ public class HttpWriterTest return null; } - @Override - protected void completed() - { - } - @Override protected void execute(Runnable task) { diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index bf46c44496c..77945a682d4 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -137,11 +137,6 @@ public class ResponseTest { } - @Override - protected void completed() - { - } - @Override public Connector getConnector() {