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
