From 307c77927412c85498327d1fab9ef91d1e11086c Mon Sep 17 00:00:00 2001 From: markt Date: Fri, 12 Aug 2011 18:24:21 +0000 Subject: [PATCH] Align OutputBuffer initialisation across the connectors Reduce visibility of properties and methods git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1157204 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/coyote/http11/AbstractOutputBuffer.java | 8 +++- .../apache/coyote/http11/Http11AprProcessor.java | 7 ++-- .../apache/coyote/http11/Http11NioProcessor.java | 5 +-- java/org/apache/coyote/http11/Http11Processor.java | 4 +- .../coyote/http11/InternalAprOutputBuffer.java | 27 +++++------- .../coyote/http11/InternalNioOutputBuffer.java | 49 +++++++++------------- .../apache/coyote/http11/InternalOutputBuffer.java | 32 +++++++------- .../apache/coyote/http11/TestGzipOutputFilter.java | 2 +- 8 files changed, 60 insertions(+), 74 deletions(-) diff --git a/java/org/apache/coyote/http11/AbstractOutputBuffer.java b/java/org/apache/coyote/http11/AbstractOutputBuffer.java index b4a434527..a709720a4 100644 --- a/java/org/apache/coyote/http11/AbstractOutputBuffer.java +++ b/java/org/apache/coyote/http11/AbstractOutputBuffer.java @@ -28,9 +28,11 @@ import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.buf.CharChunk; import org.apache.tomcat.util.buf.MessageBytes; import org.apache.tomcat.util.http.HttpMessages; +import org.apache.tomcat.util.net.AbstractEndpoint; +import org.apache.tomcat.util.net.SocketWrapper; import org.apache.tomcat.util.res.StringManager; -public abstract class AbstractOutputBuffer implements OutputBuffer{ +public abstract class AbstractOutputBuffer implements OutputBuffer{ // ----------------------------------------------------- Instance Variables @@ -316,7 +318,11 @@ public abstract class AbstractOutputBuffer implements OutputBuffer{ activeFilters[lastActiveFilter].end(); finished = true; } + + public abstract void init(SocketWrapper socketWrapper, + AbstractEndpoint endpoint) throws IOException; + public abstract void sendAck() throws IOException; protected abstract void commit() throws IOException; diff --git a/java/org/apache/coyote/http11/Http11AprProcessor.java b/java/org/apache/coyote/http11/Http11AprProcessor.java index 7d184a3ed..01f884a8f 100644 --- a/java/org/apache/coyote/http11/Http11AprProcessor.java +++ b/java/org/apache/coyote/http11/Http11AprProcessor.java @@ -175,8 +175,7 @@ public class Http11AprProcessor extends AbstractHttp11Processor { // Setting up the socket this.socket = socketWrapper; inputBuffer.init(socketWrapper, endpoint); - long socketRef = socketWrapper.getSocket().longValue(); - outputBuffer.setSocket(socketRef); + outputBuffer.init(socketWrapper, endpoint); // Error flag error = false; @@ -190,6 +189,8 @@ public class Http11AprProcessor extends AbstractHttp11Processor { boolean openSocket = false; boolean sendfileInProgress = false; + long socketRef = socketWrapper.getSocket().longValue(); + while (!error && keepAlive && !comet && !isAsync() && !endpoint.isPaused()) { // Parsing the request header @@ -624,7 +625,7 @@ public class Http11AprProcessor extends AbstractHttp11Processor { } @Override - protected AbstractOutputBuffer getOutputBuffer() { + protected AbstractOutputBuffer getOutputBuffer() { return outputBuffer; } } diff --git a/java/org/apache/coyote/http11/Http11NioProcessor.java b/java/org/apache/coyote/http11/Http11NioProcessor.java index b0078ec6e..3ac942188 100644 --- a/java/org/apache/coyote/http11/Http11NioProcessor.java +++ b/java/org/apache/coyote/http11/Http11NioProcessor.java @@ -206,8 +206,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { // Setting up the socket this.socket = socketWrapper; inputBuffer.init(socketWrapper, endpoint); - outputBuffer.setSocket(this.socket.getSocket()); - outputBuffer.setSelectorPool(((NioEndpoint)endpoint).getSelectorPool()); + outputBuffer.init(socketWrapper, endpoint); // Error flag error = false; @@ -651,7 +650,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { } @Override - protected AbstractOutputBuffer getOutputBuffer() { + protected AbstractOutputBuffer getOutputBuffer() { return outputBuffer; } diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index ef797ab2c..0bf482953 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -139,7 +139,7 @@ public class Http11Processor extends AbstractHttp11Processor { // Setting up the I/O this.socket = socketWrapper; inputBuffer.init(socketWrapper, endpoint); - outputBuffer.setOutputStream(socket.getSocket().getOutputStream()); + outputBuffer.init(socketWrapper, endpoint); // Error flag error = false; @@ -563,7 +563,7 @@ public class Http11Processor extends AbstractHttp11Processor { } @Override - protected AbstractOutputBuffer getOutputBuffer() { + protected AbstractOutputBuffer getOutputBuffer() { return outputBuffer; } diff --git a/java/org/apache/coyote/http11/InternalAprOutputBuffer.java b/java/org/apache/coyote/http11/InternalAprOutputBuffer.java index 53b331b0d..94b8a8943 100644 --- a/java/org/apache/coyote/http11/InternalAprOutputBuffer.java +++ b/java/org/apache/coyote/http11/InternalAprOutputBuffer.java @@ -26,13 +26,15 @@ import org.apache.coyote.Response; import org.apache.tomcat.jni.Socket; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.http.HttpMessages; +import org.apache.tomcat.util.net.AbstractEndpoint; +import org.apache.tomcat.util.net.SocketWrapper; /** * Output buffer. * * @author Remy Maucherat */ -public class InternalAprOutputBuffer extends AbstractOutputBuffer { +public class InternalAprOutputBuffer extends AbstractOutputBuffer { // ----------------------------------------------------------- Constructors @@ -72,30 +74,26 @@ public class InternalAprOutputBuffer extends AbstractOutputBuffer { /** * Underlying socket. */ - protected long socket; + private long socket; /** * Direct byte buffer used for writing. */ - protected ByteBuffer bbuf = null; + private ByteBuffer bbuf = null; - // ------------------------------------------------------------- Properties + // --------------------------------------------------------- Public Methods + @Override + public void init(SocketWrapper socketWrapper, + AbstractEndpoint endpoint) throws IOException { - /** - * Set the underlying socket. - */ - public void setSocket(long socket) { - this.socket = socket; + socket = socketWrapper.getSocket().longValue(); Socket.setsbb(this.socket, bbuf); } - // --------------------------------------------------------- Public Methods - - /** * Flush the response. * @@ -197,11 +195,10 @@ public class InternalAprOutputBuffer extends AbstractOutputBuffer { } - /** * Callback to write data from the buffer. */ - protected void flushBuffer() + private void flushBuffer() throws IOException { if (bbuf.position() > 0) { if (Socket.sendbb(socket, 0, bbuf.position()) < 0) { @@ -253,6 +250,4 @@ public class InternalAprOutputBuffer extends AbstractOutputBuffer { return byteCount; } } - - } diff --git a/java/org/apache/coyote/http11/InternalNioOutputBuffer.java b/java/org/apache/coyote/http11/InternalNioOutputBuffer.java index ecdd7e78b..938b7cf56 100644 --- a/java/org/apache/coyote/http11/InternalNioOutputBuffer.java +++ b/java/org/apache/coyote/http11/InternalNioOutputBuffer.java @@ -27,9 +27,11 @@ import org.apache.coyote.Response; import org.apache.tomcat.util.MutableInteger; import org.apache.tomcat.util.buf.ByteChunk; import org.apache.tomcat.util.http.HttpMessages; +import org.apache.tomcat.util.net.AbstractEndpoint; import org.apache.tomcat.util.net.NioChannel; import org.apache.tomcat.util.net.NioEndpoint; import org.apache.tomcat.util.net.NioSelectorPool; +import org.apache.tomcat.util.net.SocketWrapper; /** * Output buffer. @@ -37,7 +39,7 @@ import org.apache.tomcat.util.net.NioSelectorPool; * @author Remy Maucherat * @author Filip Hanik */ -public class InternalNioOutputBuffer extends AbstractOutputBuffer { +public class InternalNioOutputBuffer extends AbstractOutputBuffer { // ----------------------------------------------------------- Constructors @@ -68,37 +70,19 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { /** * Number of bytes last written */ - protected MutableInteger lastWrite = new MutableInteger(1); + private MutableInteger lastWrite = new MutableInteger(1); /** * Underlying socket. */ - protected NioChannel socket; + private NioChannel socket; /** * Selector pool, for blocking reads and blocking writes */ - protected NioSelectorPool pool; + private NioSelectorPool pool; - // ------------------------------------------------------------- Properties - - - /** - * Set the underlying socket. - */ - public void setSocket(NioChannel socket) { - this.socket = socket; - } - - public void setSelectorPool(NioSelectorPool pool) { - this.pool = pool; - } - - public NioSelectorPool getSelectorPool() { - return pool; - } - // --------------------------------------------------------- Public Methods @@ -178,18 +162,18 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { long writeTimeout = att.getTimeout(); Selector selector = null; try { - selector = getSelectorPool().get(); + selector = pool.get(); } catch ( IOException x ) { //ignore } try { - written = getSelectorPool().write(bytebuffer, socket, selector, writeTimeout, block,lastWrite); + written = pool.write(bytebuffer, socket, selector, writeTimeout, block,lastWrite); //make sure we are flushed do { if (socket.flush(true,selector,writeTimeout,lastWrite)) break; }while ( true ); }finally { - if ( selector != null ) getSelectorPool().put(selector); + if ( selector != null ) pool.put(selector); } if ( block ) bytebuffer.clear(); //only clear this.total = 0; @@ -199,6 +183,14 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { // ------------------------------------------------------ Protected Methods + @Override + public void init(SocketWrapper socketWrapper, + AbstractEndpoint endpoint) throws IOException { + + socket = socketWrapper.getSocket(); + pool = ((NioEndpoint)endpoint).getSelectorPool(); + } + /** * Commit the response. @@ -220,7 +212,7 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { } - int total = 0; + private int total = 0; private synchronized void addToBB(byte[] buf, int offset, int length) throws IOException { while (length > 0) { int thisTime = length; @@ -245,8 +237,7 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { /** * Callback to write data from the buffer. */ - protected void flushBuffer() - throws IOException { + private void flushBuffer() throws IOException { //prevent timeout for async, SelectionKey key = socket.getIOChannel().keyFor(socket.getPoller().getSelector()); @@ -294,6 +285,4 @@ public class InternalNioOutputBuffer extends AbstractOutputBuffer { return byteCount; } } - - } diff --git a/java/org/apache/coyote/http11/InternalOutputBuffer.java b/java/org/apache/coyote/http11/InternalOutputBuffer.java index a2e3c0857..b3e1259fd 100644 --- a/java/org/apache/coyote/http11/InternalOutputBuffer.java +++ b/java/org/apache/coyote/http11/InternalOutputBuffer.java @@ -19,17 +19,20 @@ package org.apache.coyote.http11; import java.io.IOException; import java.io.OutputStream; +import java.net.Socket; import org.apache.coyote.OutputBuffer; import org.apache.coyote.Response; import org.apache.tomcat.util.buf.ByteChunk; +import org.apache.tomcat.util.net.AbstractEndpoint; +import org.apache.tomcat.util.net.SocketWrapper; /** * Output buffer. * * @author Remy Maucherat */ -public class InternalOutputBuffer extends AbstractOutputBuffer +public class InternalOutputBuffer extends AbstractOutputBuffer implements ByteChunk.ByteOutputChannel { // ----------------------------------------------------------- Constructors @@ -58,7 +61,7 @@ public class InternalOutputBuffer extends AbstractOutputBuffer } /** - * Underlying output stream. + * Underlying output stream. Note: protected to assist with unit testing */ protected OutputStream outputStream; @@ -66,28 +69,16 @@ public class InternalOutputBuffer extends AbstractOutputBuffer /** * Socket buffer. */ - protected ByteChunk socketBuffer; + private ByteChunk socketBuffer; /** * Socket buffer (extra buffering to reduce number of packets sent). */ - protected boolean useSocketBuffer = false; + private boolean useSocketBuffer = false; /** - * Set the underlying socket output stream. - */ - public void setOutputStream(OutputStream outputStream) { - - // FIXME: Check for null ? - - this.outputStream = outputStream; - - } - - - /** * Set the socket buffer size. */ public void setSocketBuffer(int socketBufferSize) { @@ -104,6 +95,13 @@ public class InternalOutputBuffer extends AbstractOutputBuffer // --------------------------------------------------------- Public Methods + @Override + public void init(SocketWrapper socketWrapper, + AbstractEndpoint endpoint) throws IOException { + + outputStream = socketWrapper.getSocket().getOutputStream(); + } + /** * Flush the response. @@ -255,6 +253,4 @@ public class InternalOutputBuffer extends AbstractOutputBuffer return byteCount; } } - - } diff --git a/test/org/apache/coyote/http11/TestGzipOutputFilter.java b/test/org/apache/coyote/http11/TestGzipOutputFilter.java index 40d2136cb..84683f9bc 100644 --- a/test/org/apache/coyote/http11/TestGzipOutputFilter.java +++ b/test/org/apache/coyote/http11/TestGzipOutputFilter.java @@ -54,7 +54,7 @@ public class TestGzipOutputFilter { Response res = new Response(); InternalOutputBuffer iob = new InternalOutputBuffer(res, 8 * 1024); ByteArrayOutputStream bos = new ByteArrayOutputStream(); - iob.setOutputStream(bos); + iob.outputStream = bos; res.setOutputBuffer(iob); // set up GzipOutputFilter to attach to the InternalOutputBuffer -- 2.11.0