You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/10/11 20:09:35 UTC

[tomcat] branch 10.0.x updated: Refactor the APR/native connector shutdown

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/10.0.x by this push:
     new 4d3ee01  Refactor the APR/native connector shutdown
4d3ee01 is described below

commit 4d3ee01d14a330029f9fb75702f4658e744a8ae1
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Oct 11 21:07:48 2021 +0100

    Refactor the APR/native connector shutdown
    
    Hopefully reduce the possibility of a JVM crash during the connector
    shutdown.
---
 java/org/apache/tomcat/util/net/AprEndpoint.java | 46 +++++++++++++++++++-----
 webapps/docs/changelog.xml                       |  4 +++
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AprEndpoint.java b/java/org/apache/tomcat/util/net/AprEndpoint.java
index f2e3973..4f44e62 100644
--- a/java/org/apache/tomcat/util/net/AprEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AprEndpoint.java
@@ -551,11 +551,13 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
         }
         if (running) {
             running = false;
+            // Stop new connections being accepted.
             acceptor.stop(10);
+
+            // Stop the Poller calling select
             poller.stop();
-            for (SocketWrapperBase<Long> socketWrapper : connections.values()) {
-                socketWrapper.close();
-            }
+
+            // Wait for the acceptor to shutdown
             if (acceptor.getState() != AcceptorState.ENDED && !getBindOnInit()) {
                 log.warn(sm.getString("endpoint.warn.unlockAcceptorFailed", acceptor.getThreadName()));
                 // If the Acceptor is still running force
@@ -565,12 +567,40 @@ public class AprEndpoint extends AbstractEndpoint<Long,Long> implements SNICallB
                     serverSock = 0;
                 }
             }
-            // Close any sockets not in the poller performing blocking
-            // read/writes. Need to do this before destroying the poller since
-            // that will also destroy the root pool for these sockets.
-            for (Long s : connections.keySet()) {
-                Socket.shutdown(s.longValue(), Socket.APR_SHUTDOWN_READWRITE);
+
+            // Wait for Poller to stop
+            int waitMillis = 0;
+            try {
+                while (poller.pollerThread.isAlive() && waitMillis < 10000) {
+                    waitMillis++;
+                    Thread.sleep(1);
+                }
+            } catch (InterruptedException e) {
+                // Ignore
+            }
+
+            // Close the SocketWrapper for each open connection - this should
+            // trigger a IOException when the app (or container) tries to write.
+            // Use the blocking status write lock as a proxy for a lock on
+            // writing to the socket. Don't want to close it while another
+            // thread is writing as that could trigger a JVM crash.
+            for (SocketWrapperBase<Long> socketWrapper : connections.values()) {
+                WriteLock wl = ((AprSocketWrapper) socketWrapper).getBlockingStatusWriteLock();
+                wl.lock();
+                try {
+                    socketWrapper.close();
+                } finally {
+                    wl.unlock();
+                }
+            }
+
+            for (Long socket : connections.keySet()) {
+                // Close the APR Socket. Need to do this before destroying the
+                // poller since that will also destroy the root pool for these
+                // sockets.
+                Socket.shutdown(socket.longValue(), Socket.APR_SHUTDOWN_READWRITE);
             }
+
             try {
                 poller.destroy();
             } catch (Exception e) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4fadb0b..2b5e136 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -111,6 +111,10 @@
         Improve performance of Connector shutdown - primarily to reduce the time
         it takes to run the test suite. (markt)
       </scode>
+      <fix>
+        Refactor the APR/native connector shutdown to reduce the possibility of
+        a JVM crash during the connector shutdown. (markt)
+      </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