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 9b1a8b6 64007: Cancel selection key before wrapper close 9b1a8b6 is described below commit 9b1a8b67bffe462fc745b19e15ed59c37e2e1dcf Author: remm <r...@apache.org> AuthorDate: Tue Dec 17 15:12:39 2019 +0100 64007: Cancel selection key before wrapper close Due to NIO intricacies, this could cause a deadlock. At least that's what the traces in the BZ seem to show, when the wrapper close would sync on a lot of objects and then attempt to cancel a key, which would lock the poller which routinely does the same sync internally to manipulate keys. Canceling the key first without sync as it was done in 8.5 should work. --- java/org/apache/tomcat/util/net/NioEndpoint.java | 14 ++++++-------- webapps/docs/changelog.xml | 4 ++++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java index 67f02a7..9a70075 100644 --- a/java/org/apache/tomcat/util/net/NioEndpoint.java +++ b/java/org/apache/tomcat/util/net/NioEndpoint.java @@ -666,25 +666,23 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel> public void cancelledKey(SelectionKey sk, SocketWrapperBase<NioChannel> socketWrapper) { try { - if (socketWrapper != null) { - socketWrapper.close(); - } + // If is important to cancel the key first, otherwise a deadlock may occur between the + // poller select and the socket channel close which would cancel the key 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()) { - sk.channel().close(); - } } } catch (Throwable e) { ExceptionUtils.handleThrowable(e); if (log.isDebugEnabled()) { log.error(sm.getString("endpoint.debug.channelCloseFail"), e); } + } finally { + if (socketWrapper != null) { + socketWrapper.close(); + } } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index ceea555..242a34c 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -81,6 +81,10 @@ via the new <code>SSLHostConfig</code> and <code>SSLHostConfigCertificate</code> elements. (markt) </fix> + <fix> + <bug>64007</bug>: Cancel selection key in poller before wrapper close to + avoid possible deadlock. (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