This is an automated email from the ASF dual-hosted git repository. remm pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/master by this push: new c2d6278 Refactor socket wrapper close c2d6278 is described below commit c2d6278b339384f9e6679b718ceb861d0329be1d Author: remm <r...@apache.org> AuthorDate: Wed May 15 13:40:49 2019 +0200 Refactor socket wrapper close Redo again close processing using an atomic boolean and a doClose method that subclasses will implement, with a guarantee that it will be run only once. Improve slightly NIO close with respect to recycling the NioChannel. APR will use atomic boolean object for locking instead of closedLock. Once the NIOx socket wrapper is closed, the channel will now be replaced by a dummy closed channel. Also all the buffers linked will be replaced with empty ones. --- java/org/apache/tomcat/util/net/AprEndpoint.java | 72 ++++++++---------- java/org/apache/tomcat/util/net/Nio2Channel.java | 85 +++++++++++++++++++++- java/org/apache/tomcat/util/net/Nio2Endpoint.java | 37 ++++------ java/org/apache/tomcat/util/net/NioChannel.java | 52 +++++++++++-- java/org/apache/tomcat/util/net/NioEndpoint.java | 74 ++++++++----------- .../tomcat/util/net/SocketBufferHandler.java | 6 ++ .../apache/tomcat/util/net/SocketWrapperBase.java | 32 +++++++- java/org/apache/tomcat/util/net/WriteBuffer.java | 3 + webapps/docs/changelog.xml | 12 +++ 9 files changed, 254 insertions(+), 119 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index df85e6c..9551b7e 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -494,11 +494,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB running = false; poller.stop(); for (SocketWrapperBase<Long> socketWrapper : connections.values()) { - try { - socketWrapper.close(); - } catch (IOException e) { - // Ignore - } + socketWrapper.close(); } long waitLeft = 10000; while (waitLeft > 0 && @@ -2150,9 +2146,6 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB private final ByteBuffer sslOutputBuffer; - private final Object closedLock = new Object(); - private volatile boolean closed = false; - // This field should only be used by Poller#run() private int pollerFlags = 0; @@ -2246,7 +2239,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB private int fillReadBuffer(boolean block, ByteBuffer to) throws IOException { - if (closed) { + if (isClosed()) { throw new IOException(sm.getString("socket.apr.closed", getSocket())); } @@ -2343,15 +2336,18 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override - public void close() { - getEndpoint().getHandler().release(this); - synchronized (closedLock) { - // APR typically crashes if the same socket is closed twice so - // make sure that doesn't happen. - if (closed) { - return; + protected void doClose() { + try { + getEndpoint().getHandler().release(this); + } catch (Throwable e) { + ExceptionUtils.handleThrowable(e); + if (log.isDebugEnabled()) { + log.error(sm.getString("endpoint.debug.handlerRelease"), e); } - closed = true; + } + socketBufferHandler = SocketBufferHandler.EMPTY; + nonBlockingWriteBuffer.clear(); + synchronized (closed) { if (sslOutputBuffer != null) { ByteBufferUtils.cleanDirectBuffer(sslOutputBuffer); } @@ -2361,14 +2357,6 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override - public boolean isClosed() { - synchronized (closedLock) { - return closed; - } - } - - - @Override protected void writeBlockingDirect(ByteBuffer from) throws IOException { if (from.isDirect()) { super.writeBlockingDirect(from); @@ -2421,7 +2409,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void doWrite(boolean block, ByteBuffer from) throws IOException { - if (closed) { + if (isClosed()) { throw new IOException(sm.getString("socket.apr.closed", getSocket())); } @@ -2521,8 +2509,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override public void registerReadInterest() { // Make sure an already closed socket is not added to the poller - synchronized (closedLock) { - if (closed) { + synchronized (closed) { + if (isClosed()) { return; } Poller p = ((AprEndpoint) getEndpoint()).getPoller(); @@ -2536,8 +2524,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override public void registerWriteInterest() { // Make sure an already closed socket is not added to the poller - synchronized (closedLock) { - if (closed) { + synchronized (closed) { + if (isClosed()) { return; } ((AprEndpoint) getEndpoint()).getPoller().add( @@ -2561,7 +2549,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateRemoteAddr() { - if (closed) { + if (isClosed()) { return; } try { @@ -2576,7 +2564,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateRemoteHost() { - if (closed) { + if (isClosed()) { return; } try { @@ -2594,7 +2582,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateRemotePort() { - if (closed) { + if (isClosed()) { return; } try { @@ -2610,7 +2598,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateLocalName() { - if (closed) { + if (isClosed()) { return; } try { @@ -2625,7 +2613,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateLocalAddr() { - if (closed) { + if (isClosed()) { return; } try { @@ -2640,7 +2628,7 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void populateLocalPort() { - if (closed) { + if (isClosed()) { return; } try { @@ -2724,8 +2712,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB } String getSSLInfoS(int id) { - synchronized (closedLock) { - if (closed) { + synchronized (closed) { + if (isClosed()) { return null; } try { @@ -2737,8 +2725,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB } int getSSLInfoI(int id) { - synchronized (closedLock) { - if (closed) { + synchronized (closed) { + if (isClosed()) { return 0; } try { @@ -2750,8 +2738,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB } byte[] getSSLInfoB(int id) { - synchronized (closedLock) { - if (closed) { + synchronized (closed) { + if (isClosed()) { return null; } try { diff --git a/java/org/apache/tomcat/util/net/Nio2Channel.java b/java/org/apache/tomcat/util/net/Nio2Channel.java index 40eb9fb..36c5b99 100644 --- a/java/org/apache/tomcat/util/net/Nio2Channel.java +++ b/java/org/apache/tomcat/util/net/Nio2Channel.java @@ -20,6 +20,7 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.AsynchronousByteChannel; import java.nio.channels.AsynchronousSocketChannel; +import java.nio.channels.ClosedChannelException; import java.nio.channels.CompletionHandler; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; @@ -135,7 +136,7 @@ public class Nio2Channel implements AsynchronousByteChannel { @Override public String toString() { - return super.toString()+":"+this.sc.toString(); + return super.toString() + ":" + sc.toString(); } @Override @@ -213,7 +214,6 @@ public class Nio2Channel implements AsynchronousByteChannel { return DONE; } - private ApplicationBufferHandler appReadBufHandler; public void setAppReadBufHandler(ApplicationBufferHandler handler) { this.appReadBufHandler = handler; @@ -221,4 +221,85 @@ public class Nio2Channel implements AsynchronousByteChannel { protected ApplicationBufferHandler getAppReadBufHandler() { return appReadBufHandler; } + + private static final Future<Integer> DONE_INT = new Future<Integer>() { + @Override + public boolean cancel(boolean mayInterruptIfRunning) { + return false; + } + @Override + public boolean isCancelled() { + return false; + } + @Override + public boolean isDone() { + return true; + } + @Override + public Integer get() throws InterruptedException, + ExecutionException { + return Integer.valueOf(-1); + } + @Override + public Integer get(long timeout, TimeUnit unit) + throws InterruptedException, ExecutionException, + TimeoutException { + return Integer.valueOf(-1); + } + }; + + static final Nio2Channel CLOSED_NIO2_CHANNEL = new ClosedNio2Channel(); + public static class ClosedNio2Channel extends Nio2Channel { + public ClosedNio2Channel() { + super(null); + } + @Override + public void close() throws IOException { + } + @Override + public boolean isOpen() { + return false; + } + @Override + public void reset(AsynchronousSocketChannel channel, SocketWrapperBase<Nio2Channel> socket) throws IOException { + } + @Override + public void free() { + } + @Override + public Future<Integer> read(ByteBuffer dst) { + return DONE_INT; + } + @Override + public <A> void read(ByteBuffer dst, + long timeout, TimeUnit unit, A attachment, + CompletionHandler<Integer, ? super A> handler) { + handler.failed(new ClosedChannelException(), attachment); + } + @Override + public <A> void read(ByteBuffer[] dsts, + int offset, int length, long timeout, TimeUnit unit, + A attachment, CompletionHandler<Long,? super A> handler) { + handler.failed(new ClosedChannelException(), attachment); + } + @Override + public Future<Integer> write(ByteBuffer src) { + return DONE_INT; + } + @Override + public <A> void write(ByteBuffer src, long timeout, TimeUnit unit, A attachment, + CompletionHandler<Integer, ? super A> handler) { + handler.failed(new ClosedChannelException(), attachment); + } + @Override + public <A> void write(ByteBuffer[] srcs, int offset, int length, + long timeout, TimeUnit unit, A attachment, + CompletionHandler<Long,? super A> handler) { + handler.failed(new ClosedChannelException(), attachment); + } + @Override + public String toString() { + return "Closed Nio2Channel"; + } + } } diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index 50eb996..ee6a9a1 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -459,6 +459,8 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS public static class Nio2SocketWrapper extends SocketWrapperBase<Nio2Channel> { + private final SynchronizedStack<Nio2Channel> nioChannels; + private SendfileData sendfileData = null; private final CompletionHandler<Integer, ByteBuffer> readCompletionHandler; @@ -470,8 +472,6 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS private boolean writeInterest = false; // Guarded by writeCompletionHandler private boolean writeNotify = false; - private volatile boolean closed = false; - private CompletionHandler<Integer, SendfileData> sendfileHandler = new CompletionHandler<Integer, SendfileData>() { @@ -555,6 +555,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS public Nio2SocketWrapper(Nio2Channel channel, final Nio2Endpoint endpoint) { super(channel, endpoint); + nioChannels = endpoint.nioChannels; socketBufferHandler = channel.getBufHandler(); this.readCompletionHandler = new CompletionHandler<Integer, ByteBuffer>() { @@ -894,7 +895,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS @Override - public void close() { + protected void doClose() { if (log.isDebugEnabled()) { log.debug("Calling [" + getEndpoint() + "].closeSocket([" + this + "])", new Exception()); } @@ -908,19 +909,25 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } try { synchronized (getSocket()) { - if (!closed) { - closed = true; - getEndpoint().countDownConnection(); - } + getEndpoint().countDownConnection(); if (getSocket().isOpen()) { getSocket().close(true); } + socketBufferHandler = SocketBufferHandler.EMPTY; + nonBlockingWriteBuffer.clear(); + if (getEndpoint().running && !getEndpoint().paused) { + if (nioChannels == null || !nioChannels.push(getSocket())) { + getSocket().free(); + } + } } } catch (Throwable e) { ExceptionUtils.handleThrowable(e); if (log.isDebugEnabled()) { log.error(sm.getString("endpoint.debug.channelCloseFail"), e); } + } finally { + reset(Nio2Channel.CLOSED_NIO2_CHANNEL); } try { SendfileData data = getSendfileData(); @@ -936,12 +943,6 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } @Override - public boolean isClosed() { - return closed; - } - - - @Override public boolean hasAsyncIO() { return getEndpoint().getUseAsyncIO(); } @@ -1630,21 +1631,11 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS if (state == SocketState.CLOSED) { // Close socket and pool socketWrapper.close(); - if (running && !paused) { - if (nioChannels == null || !nioChannels.push(socketWrapper.getSocket())) { - socketWrapper.getSocket().free(); - } - } } else if (state == SocketState.UPGRADING) { launch = true; } } else if (handshake == -1 ) { socketWrapper.close(); - if (running && !paused) { - if (nioChannels == null || !nioChannels.push(socketWrapper.getSocket())) { - socketWrapper.getSocket().free(); - } - } } } catch (VirtualMachineError vme) { ExceptionUtils.handleThrowable(vme); diff --git a/java/org/apache/tomcat/util/net/NioChannel.java b/java/org/apache/tomcat/util/net/NioChannel.java index 01222e6..9489b72 100644 --- a/java/org/apache/tomcat/util/net/NioChannel.java +++ b/java/org/apache/tomcat/util/net/NioChannel.java @@ -102,8 +102,8 @@ public class NioChannel implements ByteChannel, ScatteringByteChannel, Gathering */ @Override public void close() throws IOException { - getIOChannel().socket().close(); - getIOChannel().close(); + sc.socket().close(); + sc.close(); } /** @@ -205,13 +205,13 @@ public class NioChannel implements ByteChannel, ScatteringByteChannel, Gathering return 0; } - public void setIOChannel(SocketChannel IOChannel) { - this.sc = IOChannel; + public void setIOChannel(SocketChannel sc) { + this.sc = sc; } @Override public String toString() { - return super.toString()+":"+this.sc.toString(); + return super.toString() + ":" + sc.toString(); } public int getOutboundRemaining() { @@ -255,4 +255,46 @@ public class NioChannel implements ByteChannel, ScatteringByteChannel, Gathering return appReadBufHandler; } + static final NioChannel CLOSED_NIO_CHANNEL = new ClosedNioChannel(); + public static class ClosedNioChannel extends NioChannel { + public ClosedNioChannel() { + super(null, null); + } + @Override + public void close() throws IOException { + } + @Override + public boolean isOpen() { + return false; + } + @Override + public void reset() throws IOException { + } + @Override + public void free() { + } + @Override + public int read(ByteBuffer dst) throws IOException { + return -1; + } + @Override + public long read(ByteBuffer[] dsts, int offset, int length) + throws IOException { + return -1L; + } + @Override + public int write(ByteBuffer src) throws IOException { + checkInterruptStatus(); + return -1; + } + @Override + public long write(ByteBuffer[] srcs, int offset, int length) + throws IOException { + return -1L; + } + @Override + public String toString() { + return "Closed NioChannel"; + } + } } diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 8449bf0..f31a508 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -466,31 +466,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> return new SocketProcessor(socketWrapper, event); } - - private void close(NioChannel socket, SelectionKey key) { - try { - Poller poller = this.poller; - if (poller != null && poller.cancelledKey(key) != null) { - // SocketWrapper (attachment) was removed from the - // key - recycle the key. This can only happen once - // per attempted closure so it is used to determine - // whether or not to return the key to the cache. - // We do NOT want to do this more than once - see BZ - // 57340 / 57943. - if (log.isDebugEnabled()) { - log.debug("Socket: [" + socket + "] closed"); - } - if (running && !paused) { - if (nioChannels == null || !nioChannels.push(socket)) { - socket.free(); - } - } - } - } catch (Exception x) { - log.error(sm.getString("endpoint.err.close"), x); - } - } - // ----------------------------------------------------- Poller Inner Classes /** @@ -886,7 +861,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> if (log.isDebugEnabled()) { log.debug("Send file connection is being closed"); } - close(sc, sk); + poller.cancelledKey(sk); + socketWrapper.close(); break; } case PIPELINED: { @@ -894,7 +870,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> log.debug("Connection is keep alive, processing pipe-lined data"); } if (!processSocket(socketWrapper, SocketEvent.OPEN_READ, true)) { - close(sc, sk); + poller.cancelledKey(sk); + socketWrapper.close(); } break; } @@ -924,13 +901,15 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> log.debug("Unable to complete sendfile request:", e); } if (!calledByProcessor && sc != null) { - close(sc, sk); + poller.cancelledKey(sk); + socketWrapper.close(); } return SendfileState.ERROR; } catch (Throwable t) { log.error(sm.getString("endpoint.sendfile.error"), t); if (!calledByProcessor && sc != null) { - close(sc, sk); + poller.cancelledKey(sk); + socketWrapper.close(); } return SendfileState.ERROR; } @@ -1036,6 +1015,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> public static class NioSocketWrapper extends SocketWrapperBase<NioChannel> { private final NioSelectorPool pool; + private final SynchronizedStack<NioChannel> nioChannels; private Poller poller = null; private int interestOps = 0; @@ -1044,12 +1024,12 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> private volatile SendfileData sendfileData = null; private volatile long lastRead = System.currentTimeMillis(); private volatile long lastWrite = lastRead; - private volatile boolean closed = false; public NioSocketWrapper(NioChannel channel, NioEndpoint endpoint) { super(channel, endpoint); pool = endpoint.getSelectorPool(); socketBufferHandler = channel.getBufHandler(); + nioChannels = endpoint.nioChannels; } public Poller getPoller() { return poller; } @@ -1184,7 +1164,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> @Override - public void close() { + protected void doClose() { if (log.isDebugEnabled()) { log.debug("Calling [" + getEndpoint() + "].closeSocket([" + this + "])", new Exception()); } @@ -1198,19 +1178,25 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } try { synchronized (getSocket()) { - if (!closed) { - closed = true; - getEndpoint().countDownConnection(); - } + getEndpoint().countDownConnection(); if (getSocket().isOpen()) { getSocket().close(true); } + socketBufferHandler = SocketBufferHandler.EMPTY; + nonBlockingWriteBuffer.clear(); + if (getEndpoint().running && !getEndpoint().paused) { + if (nioChannels == null || !nioChannels.push(getSocket())) { + getSocket().free(); + } + } } } catch (Throwable e) { ExceptionUtils.handleThrowable(e); if (log.isDebugEnabled()) { log.error(sm.getString("endpoint.debug.channelCloseFail"), e); } + } finally { + reset(NioChannel.CLOSED_NIO_CHANNEL); } try { SendfileData data = getSendfileData(); @@ -1225,13 +1211,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } } - - @Override - public boolean isClosed() { - return closed; - } - - private int fillReadBuffer(boolean block) throws IOException { socketBufferHandler.configureReadBufferForWrite(); return fillReadBuffer(block, socketBufferHandler.getReadBuffer()); @@ -1540,6 +1519,11 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> protected void doRun() { NioChannel socket = socketWrapper.getSocket(); SelectionKey key = socket.getIOChannel().keyFor(socket.getSocketWrapper().getPoller().getSelector()); + Poller poller = NioEndpoint.this.poller; + if (poller == null) { + socketWrapper.close(); + return; + } try { int handshake = -1; @@ -1582,10 +1566,12 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> state = getHandler().process(socketWrapper, event); } if (state == SocketState.CLOSED) { - close(socket, key); + poller.cancelledKey(key); + socketWrapper.close(); } } else if (handshake == -1 ) { - close(socket, key); + poller.cancelledKey(key); + socketWrapper.close(); } else if (handshake == SelectionKey.OP_READ){ socketWrapper.registerReadInterest(); } else if (handshake == SelectionKey.OP_WRITE){ diff --git a/java/org/apache/tomcat/util/net/SocketBufferHandler.java b/java/org/apache/tomcat/util/net/SocketBufferHandler.java index 134ee1b..89fa86e 100644 --- a/java/org/apache/tomcat/util/net/SocketBufferHandler.java +++ b/java/org/apache/tomcat/util/net/SocketBufferHandler.java @@ -22,6 +22,12 @@ import org.apache.tomcat.util.buf.ByteBufferUtils; public class SocketBufferHandler { + static SocketBufferHandler EMPTY = new SocketBufferHandler(0, 0, false) { + @Override + public void expand(int newSize) { + } + }; + private volatile boolean readBufferConfiguredForWrite = true; private volatile ByteBuffer readBuffer; diff --git a/java/org/apache/tomcat/util/net/SocketWrapperBase.java b/java/org/apache/tomcat/util/net/SocketWrapperBase.java index b42c603..01502cb 100644 --- a/java/org/apache/tomcat/util/net/SocketWrapperBase.java +++ b/java/org/apache/tomcat/util/net/SocketWrapperBase.java @@ -28,6 +28,7 @@ import java.util.concurrent.Executor; import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.Semaphore; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock.WriteLock; @@ -43,9 +44,11 @@ public abstract class SocketWrapperBase<E> { protected static final StringManager sm = StringManager.getManager(SocketWrapperBase.class); - private final E socket; + private E socket; private final AbstractEndpoint<E,?> endpoint; + protected final AtomicBoolean closed = new AtomicBoolean(false); + // Volatile because I/O and setting the timeout values occurs on a different // thread to the thread checking the timeout. private volatile long readTimeout = -1; @@ -124,6 +127,10 @@ public abstract class SocketWrapperBase<E> { return socket; } + protected void reset(E closedSocket) { + socket = closedSocket; + } + protected AbstractEndpoint<E,?> getEndpoint() { return endpoint; } @@ -379,8 +386,27 @@ public abstract class SocketWrapperBase<E> { } - public abstract void close() throws IOException; - public abstract boolean isClosed(); + /** + * Close the socket wrapper. + */ + public void close() { + if (closed.compareAndSet(false, true)) { + doClose(); + } + } + + /** + * Perform the actual close. The closed atomic boolean guarantees this will + * be called only once per wrapper. + */ + protected abstract void doClose(); + + /** + * @return true if the wrapper has been closed + */ + public boolean isClosed() { + return closed.get(); + } /** diff --git a/java/org/apache/tomcat/util/net/WriteBuffer.java b/java/org/apache/tomcat/util/net/WriteBuffer.java index 4a1d2c5..e4d28cb 100644 --- a/java/org/apache/tomcat/util/net/WriteBuffer.java +++ b/java/org/apache/tomcat/util/net/WriteBuffer.java @@ -43,6 +43,9 @@ public class WriteBuffer { this.bufferSize = bufferSize; } + void clear() { + buffers.clear(); + } void add(byte[] buf, int offset, int length) { ByteBufferHolder holder = getByteBufferHolder(length); diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 459c293..0a0d1e3 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -112,6 +112,18 @@ Refactor async IO implementation to the <code>SocketWrapperBase</code>. (remm) </scode> + <update> + Refactor <code>SocketWrapperBase</code> close using an atomic boolean + and a <code>doClose</code> method that subclasses will implement, with + a guarantee that it will be run only once. (remm) + </update> + <fix> + Decouple the socket wrapper, which is not recycled, from the NIOx + channel after close, and replace it with a dummy static object. (remm) + </fix> + <fix> + Clear buffers on socket wrapper close. (remm) + </fix> </changelog> </subsection> <subsection name="Other"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org