This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch 8.5.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 78b216839fd27dd92994652dd094bb8cf9612eca Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Feb 18 14:13:16 2022 +0000 s --- java/org/apache/tomcat/util/net/AprEndpoint.java | 57 ++++++------ java/org/apache/tomcat/util/net/Nio2Endpoint.java | 34 +++++-- java/org/apache/tomcat/util/net/NioEndpoint.java | 106 +++++++--------------- 3 files changed, 83 insertions(+), 114 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java index 5145432..3fa5a2d 100644 --- a/java/org/apache/tomcat/util/net/AprEndpoint.java +++ b/java/org/apache/tomcat/util/net/AprEndpoint.java @@ -765,37 +765,34 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB */ protected boolean processSocketWithOptions(long socket) { try { - // During shutdown, executor may be null - avoid NPE - if (running) { - if (log.isDebugEnabled()) { - log.debug(sm.getString("endpoint.debug.socket", - Long.valueOf(socket))); - } - - // Do the duplicate accept check here rather than in Acceptor.run() - // so we can cache the results in the SocketWrapper - AprSocketWrapper wrapper = new AprSocketWrapper(Long.valueOf(socket), this); - // Bug does not affect Windows. Skip the check on that platform. - if (!JrePlatform.IS_WINDOWS) { - long currentNanoTime = System.nanoTime(); - if (wrapper.getRemotePort() == previousAcceptedPort) { - if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) { - if (currentNanoTime - previousAcceptedSocketNanoTime < 1000) { - throw new IOException(sm.getString("endpoint.err.duplicateAccept")); - } + if (log.isDebugEnabled()) { + log.debug(sm.getString("endpoint.debug.socket", socket)); + } + + // Do the duplicate accept check here rather than in Acceptor.run() + // so we can cache the results in the SocketWrapper + AprSocketWrapper wrapper = new AprSocketWrapper(Long.valueOf(socket), this); + // Bug does not affect Windows. Skip the check on that platform. + if (!JrePlatform.IS_WINDOWS) { + long currentNanoTime = System.nanoTime(); + if (wrapper.getRemotePort() == previousAcceptedPort) { + if (wrapper.getRemoteAddr().equals(previousAcceptedAddress)) { + if (currentNanoTime - previousAcceptedSocketNanoTime < 1000) { + throw new IOException(sm.getString("endpoint.err.duplicateAccept")); } } - previousAcceptedPort = wrapper.getRemotePort(); - previousAcceptedAddress = wrapper.getRemoteAddr(); - previousAcceptedSocketNanoTime = currentNanoTime; } - - wrapper.setKeepAliveLeft(getMaxKeepAliveRequests()); - wrapper.setReadTimeout(getConnectionTimeout()); - wrapper.setWriteTimeout(getConnectionTimeout()); - connections.put(Long.valueOf(socket), wrapper); - getExecutor().execute(new SocketWithOptionsProcessor(wrapper)); + previousAcceptedPort = wrapper.getRemotePort(); + previousAcceptedAddress = wrapper.getRemoteAddr(); + previousAcceptedSocketNanoTime = currentNanoTime; } + + connections.put(Long.valueOf(socket), wrapper); + wrapper.setKeepAliveLeft(getMaxKeepAliveRequests()); + wrapper.setReadTimeout(getConnectionTimeout()); + wrapper.setWriteTimeout(getConnectionTimeout()); + getExecutor().execute(new SocketWithOptionsProcessor(wrapper)); + return true; } catch (RejectedExecutionException x) { log.warn(sm.getString("endpoint.rejectedExecution", socket), x); } catch (Throwable t) { @@ -803,9 +800,8 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB // This means we got an OOM or similar creating a thread, or that // the pool and its queue are full log.error(sm.getString("endpoint.process.fail"), t); - return false; } - return true; + return false; } @@ -2437,6 +2433,9 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB @Override protected void doClose() { + if (log.isDebugEnabled()) { + log.debug("Calling [" + getEndpoint() + "].closeSocket([" + this + "])"); + } try { getEndpoint().getHandler().release(this); } catch (Throwable e) { diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index e9c234f..217cdb8 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -532,8 +532,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS break; } case PIPELINED: { - getEndpoint().processSocket(Nio2SocketWrapper.this, - SocketEvent.OPEN_READ, true); + if (!getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_READ, true)) { + close(); + } break; } case OPEN: { @@ -627,9 +628,10 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS // notify/dispatch to do the release. readPending.release(); // If already closed, don't call onError and close again - return; + getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.STOP, false); + } else if (!getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true)) { + close(); } - getEndpoint().processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true); } }; @@ -666,7 +668,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } } if (notify) { - endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_WRITE, true); + if (!endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_WRITE, true)) { + close(); + } } } @Override @@ -679,7 +683,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } setError(ioe); writePending.release(); - endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true); + if (!endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true)) { + close(); + } } }; @@ -712,7 +718,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } } if (notify) { - endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_WRITE, true); + if (!endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.OPEN_WRITE, true)) { + close(); + } } } @Override @@ -725,7 +733,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS } setError(ioe); writePending.release(); - endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true); + if (!endpoint.processSocket(Nio2SocketWrapper.this, SocketEvent.ERROR, true)) { + close(); + } } }; @@ -1392,7 +1402,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS if (fillReadBuffer(false) > 0) { // Special case where the read completed inline, there is no notification // in that case so it has to be done here - getEndpoint().processSocket(this, SocketEvent.OPEN_READ, true); + if (!getEndpoint().processSocket(this, SocketEvent.OPEN_READ, true)) { + close(); + } } } catch (IOException e) { // Will never happen @@ -1416,7 +1428,9 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS writeInterest = true; if (writePending.availablePermits() == 1) { // If no write is pending, notify that writing is possible - getEndpoint().processSocket(this, SocketEvent.OPEN_WRITE, true); + if (!getEndpoint().processSocket(this, SocketEvent.OPEN_WRITE, true)) { + close(); + } } } } diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 165aa2f..9813467 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -609,7 +609,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> return new SocketProcessor(socketWrapper, event); } - // ----------------------------------------------------- Poller Inner Classes /** @@ -757,10 +756,10 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> attachment.interestOps(ops); key.interestOps(ops); } catch (CancelledKeyException ckx) { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } else { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } } @@ -801,59 +800,21 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> addEvent(event); } - public NioSocketWrapper cancelledKey(SelectionKey sk) { - NioSocketWrapper ka = null; + public void cancelledKey(SelectionKey sk, SocketWrapperBase<NioChannel> socketWrapper) { try { - if ( sk == null ) - { - return null;//nothing to do - } - ka = (NioSocketWrapper) sk.attach(null); - if (ka != null) { - // If attachment is non-null then there may be a current - // connection with an associated processor. - getHandler().release(ka); - } - if (sk.isValid()) { - sk.cancel(); + if (socketWrapper != null) { + socketWrapper.close(); } - // If it is available, close the NioChannel first which should - // in turn close the underlying SocketChannel. The NioChannel - // needs to be closed first, if available, to ensure that TLS - // connections are shut down cleanly. - if (ka != null) { - try { - ka.getSocket().close(true); - } catch (Exception e){ - if (log.isDebugEnabled()) { - log.debug(sm.getString( - "endpoint.debug.socketCloseFail"), e); - } + if (sk != null) { + sk.attach(null); + if (sk.isValid()) { + sk.cancel(); } - } - // The SocketChannel is also available via the SelectionKey. If - // it hasn't been closed in the block above, close it now. - if (sk.channel().isOpen()) { - try { + // The SocketChannel is also available via the SelectionKey. If + // it hasn't been closed in the block above, close it now. + if (sk.channel().isOpen()) { sk.channel().close(); - } catch (Exception e) { - if (log.isDebugEnabled()) { - log.debug(sm.getString( - "endpoint.debug.channelCloseFail"), e); - } - } - } - try { - if (ka != null && ka.getSendfileData() != null - && ka.getSendfileData().fchannel != null - && ka.getSendfileData().fchannel.isOpen()) { - ka.getSendfileData().fchannel.close(); } - } catch (Exception ignore) { - } - if (ka != null) { - countDownConnection(); - ka.closed.set(true); } } catch (Throwable e) { ExceptionUtils.handleThrowable(e); @@ -861,7 +822,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> log.error(sm.getString("endpoint.debug.channelCloseFail"), e); } } - return ka; } /** @@ -933,7 +893,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> protected void processKey(SelectionKey sk, NioSocketWrapper socketWrapper) { try { if (close) { - cancelledKey(sk); + cancelledKey(sk, socketWrapper); } else if (sk.isValid() && socketWrapper != null ) { if (sk.isReadable() || sk.isWritable() ) { if (socketWrapper.getSendfileData() != null ) { @@ -953,16 +913,16 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } } if (closeSocket) { - cancelledKey(sk); + cancelledKey(sk, socketWrapper); } } } } else { // Invalid key - cancelledKey(sk); + cancelledKey(sk, socketWrapper); } } catch (CancelledKeyException ckx) { - cancelledKey(sk); + cancelledKey(sk, socketWrapper); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.error(sm.getString("endpoint.nio.keyProcessingError"), t); @@ -1030,8 +990,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> if (log.isDebugEnabled()) { log.debug("Send file connection is being closed"); } - poller.cancelledKey(sk); - socketWrapper.close(); + poller.cancelledKey(sk, socketWrapper); break; } case PIPELINED: { @@ -1039,8 +998,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> log.debug("Connection is keep alive, processing pipe-lined data"); } if (!processSocket(socketWrapper, SocketEvent.OPEN_READ, true)) { - poller.cancelledKey(sk); - socketWrapper.close(); + poller.cancelledKey(sk, socketWrapper); } break; } @@ -1070,15 +1028,13 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> log.debug("Unable to complete sendfile request:", e); } if (!calledByProcessor && sc != null) { - poller.cancelledKey(sk); - socketWrapper.close(); + poller.cancelledKey(sk, socketWrapper); } return SendfileState.ERROR; } catch (Throwable t) { log.error(sm.getString("endpoint.sendfile.error"), t); if (!calledByProcessor && sc != null) { - poller.cancelledKey(sk); - socketWrapper.close(); + poller.cancelledKey(sk, socketWrapper); } return SendfileState.ERROR; } @@ -1114,7 +1070,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> try { if (socketWrapper == null) { // We don't support any keys without attachments - cancelledKey(key); + cancelledKey(key, null); } else if (close) { key.interestOps(0); // Avoid duplicate stop calls @@ -1147,19 +1103,19 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> socketWrapper.setError(new SocketTimeoutException()); if (readTimeout && socketWrapper.readOperation != null) { if (!socketWrapper.readOperation.process()) { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } else if (writeTimeout && socketWrapper.writeOperation != null) { if (!socketWrapper.writeOperation.process()) { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } else if (!processSocket(socketWrapper, SocketEvent.ERROR, true)) { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } } } catch (CancelledKeyException ckx) { - cancelledKey(key); + cancelledKey(key, socketWrapper); } } } catch (ConcurrentModificationException cme) { @@ -1757,23 +1713,23 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> state = getHandler().process(socketWrapper, event); } if (state == SocketState.CLOSED) { - poller.cancelledKey(key); - socketWrapper.close(); } + poller.cancelledKey(key, socketWrapper); + } } else if (handshake == -1 ) { getHandler().process(socketWrapper, SocketEvent.CONNECT_FAIL); - poller.cancelledKey(key); - socketWrapper.close(); } else if (handshake == SelectionKey.OP_READ){ + poller.cancelledKey(key, socketWrapper); + } else if (handshake == SelectionKey.OP_READ) { socketWrapper.registerReadInterest(); } else if (handshake == SelectionKey.OP_WRITE){ socketWrapper.registerWriteInterest(); } } catch (CancelledKeyException cx) { - socket.getPoller().cancelledKey(key); + socket.getPoller().cancelledKey(key, socketWrapper); } catch (VirtualMachineError vme) { ExceptionUtils.handleThrowable(vme); } catch (Throwable t) { log.error("", t); - socket.getPoller().cancelledKey(key); + socket.getPoller().cancelledKey(key, socketWrapper); } finally { socketWrapper = null; event = null; --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org