You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2019/12/17 14:12:54 UTC

[tomcat] branch master updated: 64007: Cancel selection key before wrapper close

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 <re...@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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org