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 5f4d5b3 Better cleanup in setSocketOptions 5f4d5b3 is described below commit 5f4d5b357f4388149845fa96d87930d6f664e252 Author: remm <r...@apache.org> AuthorDate: Thu Nov 7 11:10:41 2019 +0100 Better cleanup in setSocketOptions Since the connections map is updated here, the socket must be removed from it if things go wrong before the wrapper processing begins. Also call free() on the channel (and then discard it) since this wouldn't be done anywhere and could leak direct memory in some cases. --- java/org/apache/tomcat/util/net/Nio2Endpoint.java | 18 ++++++++++++------ java/org/apache/tomcat/util/net/NioEndpoint.java | 15 ++++++++++----- webapps/docs/changelog.xml | 3 +++ 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java index 6ecba6a..5879fa9 100644 --- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java +++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java @@ -302,9 +302,10 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS */ @Override protected boolean setSocketOptions(AsynchronousSocketChannel socket) { + Nio2Channel channel = null; + boolean success = false; try { socketProperties.setProperties(socket); - Nio2Channel channel = null; if (nioChannels != null) { channel = nioChannels.pop(); } @@ -326,14 +327,19 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS socketWrapper.setWriteTimeout(getConnectionTimeout()); socketWrapper.setKeepAliveLeft(Nio2Endpoint.this.getMaxKeepAliveRequests()); socketWrapper.setSecure(isSSLEnabled()); - // Continue processing on another thread - return processSocket(socketWrapper, SocketEvent.OPEN_READ, false); + // Continue processing on the same thread as the acceptor is async + success = processSocket(socketWrapper, SocketEvent.OPEN_READ, false); } catch (Throwable t) { ExceptionUtils.handleThrowable(t); - log.error(sm.getString("endpoint.socketOptionsError"),t); + log.error(sm.getString("endpoint.socketOptionsError"), t); + } finally { + if (!success && channel != null) { + connections.remove(channel); + channel.free(); + } } - // Tell to close the socket - return false; + // Tell to close the socket if needed + return success; } diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index d059a70..1e5b900 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -393,6 +393,8 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> */ @Override protected boolean setSocketOptions(SocketChannel socket) { + NioChannel channel = null; + boolean success = false; // Process the connection try { // Disable blocking, polling will be used @@ -400,7 +402,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> Socket sock = socket.socket(); socketProperties.setProperties(sock); - NioChannel channel = null; if (nioChannels != null) { channel = nioChannels.pop(); } @@ -414,7 +415,6 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } else { channel = new NioChannel(bufhandler); } - } else { } NioSocketWrapper socketWrapper = new NioSocketWrapper(channel, this); connections.put(channel, socketWrapper); @@ -424,7 +424,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> socketWrapper.setKeepAliveLeft(NioEndpoint.this.getMaxKeepAliveRequests()); socketWrapper.setSecure(isSSLEnabled()); poller.register(channel, socketWrapper); - return true; + success = true; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); try { @@ -432,9 +432,14 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> } catch (Throwable tt) { ExceptionUtils.handleThrowable(tt); } + } finally { + if (!success && channel != null) { + connections.remove(channel); + channel.free(); + } } - // Tell to close the socket - return false; + // Tell to close the socket if needed + return success; } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 0dfa3f2..926f6ce 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -116,6 +116,9 @@ <code>certificateVerificationDepth</code> are correctly passed to the OpenSSL based SSLEngine implementation. (remm/markt) </fix> + <fix> + Improve cleanup after errors when setting socket options. (remm) + </fix> </changelog> </subsection> <subsection name="Jasper"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org