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 2024/04/19 19:08:20 UTC

(tomcat) branch 9.0.x updated (31fc71858f -> 725467428b)

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

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


    from 31fc71858f Dispatch after AsyncListener.onError() should actually dispatch
     new acc9a94f89 Fix typos
     new 3f9b18d778 Allow any positive value for socket.unlockTimeout rather than >=2s
     new 3d81c5cd9e Refactor to specify acceptor stop wait time in ms rather than s
     new 725467428b Reduce the default wait time for the acceptor unlock

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/tomcat/util/net/AbstractEndpoint.java   | 12 ++++--------
 java/org/apache/tomcat/util/net/Acceptor.java           | 12 ++++++++++--
 java/org/apache/tomcat/util/net/LocalStrings.properties |  2 ++
 java/org/apache/tomcat/util/net/Nio2Endpoint.java       |  6 +++---
 java/org/apache/tomcat/util/net/NioEndpoint.java        |  7 ++++++-
 java/org/apache/tomcat/util/net/SocketProperties.java   | 13 ++++++++++++-
 webapps/docs/changelog.xml                              | 11 +++++++++++
 webapps/docs/config/http.xml                            |  7 +++++--
 8 files changed, 53 insertions(+), 17 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 02/04: Allow any positive value for socket.unlockTimeout rather than >=2s

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 3f9b18d7788300571178b6d3f95e96fdbd80504b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 19 17:49:26 2024 +0100

    Allow any positive value for socket.unlockTimeout rather than >=2s
    
    Implement limit in setter so it always applies.
---
 java/org/apache/tomcat/util/net/AbstractEndpoint.java   |  6 +-----
 java/org/apache/tomcat/util/net/LocalStrings.properties |  2 ++
 java/org/apache/tomcat/util/net/SocketProperties.java   | 13 ++++++++++++-
 webapps/docs/changelog.xml                              |  5 +++++
 webapps/docs/config/http.xml                            |  7 +++++--
 5 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 19edc2f514..b047a5490e 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1183,13 +1183,9 @@ public abstract class AbstractEndpoint<S,U> {
 
             try (java.net.Socket s = new java.net.Socket()) {
                 int stmo = 2 * 1000;
-                int utmo = 2 * 1000;
                 if (getSocketProperties().getSoTimeout() > stmo) {
                     stmo = getSocketProperties().getSoTimeout();
                 }
-                if (getSocketProperties().getUnlockTimeout() > utmo) {
-                    utmo = getSocketProperties().getUnlockTimeout();
-                }
                 s.setSoTimeout(stmo);
                 // Newer MacOS versions (e.g. Ventura 13.2) appear to linger for ~1s on close when linger is disabled.
                 // That causes delays when running the unit tests. Explicitly enabling linger but with a timeout of
@@ -1198,7 +1194,7 @@ public abstract class AbstractEndpoint<S,U> {
                 if (getLog().isTraceEnabled()) {
                     getLog().trace("About to unlock socket for:" + unlockAddress);
                 }
-                s.connect(unlockAddress,utmo);
+                s.connect(unlockAddress, getSocketProperties().getUnlockTimeout());
                 if (getDeferAccept()) {
                     /*
                      * In the case of a deferred accept / accept filters we need to
diff --git a/java/org/apache/tomcat/util/net/LocalStrings.properties b/java/org/apache/tomcat/util/net/LocalStrings.properties
index 0945510492..10b4fc1f47 100644
--- a/java/org/apache/tomcat/util/net/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/LocalStrings.properties
@@ -161,6 +161,8 @@ socket.apr.write.error=Unexpected error [{0}] writing data to the APR/native soc
 socket.closed=The socket associated with this connection has been closed.
 socket.sslreneg=Exception re-negotiating SSL connection
 
+socketProperties.negativeUnlockTimeout=The negative value for unlockTimeout has been ignored
+
 socketWrapper.readTimeout=Read timeout
 socketWrapper.writeTimeout=Write timeout
 
diff --git a/java/org/apache/tomcat/util/net/SocketProperties.java b/java/org/apache/tomcat/util/net/SocketProperties.java
index b08325a20c..d71f6bffb8 100644
--- a/java/org/apache/tomcat/util/net/SocketProperties.java
+++ b/java/org/apache/tomcat/util/net/SocketProperties.java
@@ -26,6 +26,10 @@ import java.nio.channels.AsynchronousSocketChannel;
 
 import javax.management.ObjectName;
 
+import org.apache.juli.logging.Log;
+import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.res.StringManager;
+
 /**
  * Properties that can be set in the &lt;Connector&gt; element
  * in server.xml. All properties are prefixed with &quot;socket.&quot;
@@ -33,6 +37,9 @@ import javax.management.ObjectName;
  */
 public class SocketProperties {
 
+    private static final Log log = LogFactory.getLog(SocketProperties.class);
+    private static final StringManager sm = StringManager.getManager(SocketProperties.class);
+
     /**
      * Enable/disable socket processor cache, this bounded cache stores
      * SocketProcessor objects to reduce GC
@@ -462,7 +469,11 @@ public class SocketProperties {
     }
 
     public void setUnlockTimeout(int unlockTimeout) {
-        this.unlockTimeout = unlockTimeout;
+        if (unlockTimeout > 0) {
+            this.unlockTimeout = unlockTimeout;
+        } else {
+            log.warn(sm.getString("socketProperties.negativeUnlockTimeout"));
+        }
     }
 
     void setObjectName(ObjectName oname) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 8bb08ecb3c..c3f8e97f95 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -143,6 +143,11 @@
         Align non-secure and secure writes with NIO and skip the write attempt
         when there are no bytes to be written. (markt)
       </fix>
+      <fix>
+        Allow any positive value for <code>socket.unlockTimeout</code>. If a
+        negative or zero value is configured, the default of <code>250ms</code>
+        will be used. (mark)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index 253fed0314..c6beb0e130 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -843,8 +843,11 @@
         be used for all three.</p>
       </attribute>
       <attribute name="socket.unlockTimeout" required="false">
-        <p>(int) The timeout for a socket unlock. When a connector is stopped, it will try to release the acceptor thread by opening a connector to itself.
-           The default value is <code>250</code> and the value is in milliseconds</p>
+        <p>(int) The timeout for a socket unlock. When a connector is stopped,
+        it will try to release the acceptor thread by opening a connector to
+        itself. The default value is <code>250</code> and the value is in
+        milliseconds. This vaoue must be positive. Negative or zero values will
+        be ignored.</p>
       </attribute>
     </attributes>
   </subsection>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 03/04: Refactor to specify acceptor stop wait time in ms rather than s

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 3d81c5cd9e3836be806b651b0f81dc68f58fc4f4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 19 18:04:36 2024 +0100

    Refactor to specify acceptor stop wait time in ms rather than s
---
 java/org/apache/tomcat/util/net/AbstractEndpoint.java |  2 +-
 java/org/apache/tomcat/util/net/Acceptor.java         | 12 ++++++++++--
 java/org/apache/tomcat/util/net/Nio2Endpoint.java     |  6 +++---
 java/org/apache/tomcat/util/net/NioEndpoint.java      |  2 +-
 4 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index b047a5490e..d6dfe75f5f 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1562,7 +1562,7 @@ public abstract class AbstractEndpoint<S,U> {
     public final void closeServerSocketGraceful() {
         if (bindState == BindState.BOUND_ON_START) {
             // Stop accepting new connections
-            acceptor.stop(-1);
+            acceptor.stopMillis(-1);
             // Release locks that may be preventing the acceptor from stopping
             releaseConnectionLatch();
             unlockAccept();
diff --git a/java/org/apache/tomcat/util/net/Acceptor.java b/java/org/apache/tomcat/util/net/Acceptor.java
index bbfd8dd695..3b5cf500a3 100644
--- a/java/org/apache/tomcat/util/net/Acceptor.java
+++ b/java/org/apache/tomcat/util/net/Acceptor.java
@@ -200,12 +200,20 @@ public class Acceptor<U> implements Runnable {
      *
      * @param waitSeconds The time to wait in seconds. Use a value less than
      *                    zero for no wait.
+     *
+     * @deprecated Unused. Will be remove in Tomcat 11 onwards.
      */
+    @Deprecated
     public void stop(int waitSeconds) {
+        stopMillis(waitSeconds * 1000);
+    }
+
+
+    public void stopMillis(int waitMilliseconds) {
         stopCalled = true;
-        if (waitSeconds > 0) {
+        if (waitMilliseconds > 0) {
             try {
-                if (!stopLatch.await(waitSeconds, TimeUnit.SECONDS)) {
+                if (!stopLatch.await(waitMilliseconds, TimeUnit.MILLISECONDS)) {
                    log.warn(sm.getString("acceptor.stop.fail", getThreadName()));
                 }
             } catch (InterruptedException e) {
diff --git a/java/org/apache/tomcat/util/net/Nio2Endpoint.java b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
index 21b9f8acfb..0c8848bb98 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -210,7 +210,7 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
         }
         if (running) {
             running = false;
-            acceptor.stop(10);
+            acceptor.stopMillis(10);
             // Use the executor to avoid binding the main thread if something bad
             // occurs and unbind will also wait for a bit for it to complete
             getExecutor().execute(() -> {
@@ -448,11 +448,11 @@ public class Nio2Endpoint extends AbstractJsseEndpoint<Nio2Channel,AsynchronousS
         /**
          * Signals the Acceptor to stop.
          *
-         * @param waitSeconds Ignored for NIO2.
+         * @param waitMilliseconds Ignored for NIO2.
          *
          */
         @Override
-        public void stop(int waitSeconds) {
+        public void stopMillis(int waitMilliseconds) {
             acceptor.state = AcceptorState.ENDED;
         }
 
diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 777019c0b3..434ce6d889 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -323,7 +323,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
         }
         if (running) {
             running = false;
-            acceptor.stop(10);
+            acceptor.stopMillis(10000);
             if (poller != null) {
                 poller.destroy();
                 poller = null;


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 04/04: Reduce the default wait time for the acceptor unlock

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 725467428b4cfe14b325168bf9849f3d5fe4ae9d
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 19 18:08:09 2024 +0100

    Reduce the default wait time for the acceptor unlock
---
 java/org/apache/tomcat/util/net/NioEndpoint.java | 7 ++++++-
 webapps/docs/changelog.xml                       | 6 ++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 434ce6d889..99966fd464 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -323,7 +323,12 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
         }
         if (running) {
             running = false;
-            acceptor.stopMillis(10000);
+            /*
+             * Need to wait for the acceptor to unlock but not too long. 100ms plus twice the unlock timeout should be
+             * plenty of time for the acceptor to unlock without being an excessively long wait if the unlock fails.
+             */
+            int acceptorWaitMilliSeconds = 100 + 2 * getSocketProperties().getUnlockTimeout();
+            acceptor.stopMillis(acceptorWaitMilliSeconds);
             if (poller != null) {
                 poller.destroy();
                 poller = null;
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index c3f8e97f95..3d11e31374 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -148,6 +148,12 @@
         negative or zero value is configured, the default of <code>250ms</code>
         will be used. (mark)
       </fix>
+      <fix>
+        Reduce the time spent waiting for the connector to unlock. The previous
+        default of 10s was noticeably too long for cases where the unlock has
+        failed. The wait time is now 100ms plus twice
+        <code>socket.unlockTimeout</code>. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


(tomcat) 01/04: Fix typos

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit acc9a94f89a1c3a6e6829c0bcb1c732e59df7bf1
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 19 17:44:32 2024 +0100

    Fix typos
---
 java/org/apache/tomcat/util/net/AbstractEndpoint.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 53a0b807b3..19edc2f514 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1192,7 +1192,7 @@ public abstract class AbstractEndpoint<S,U> {
                 }
                 s.setSoTimeout(stmo);
                 // Newer MacOS versions (e.g. Ventura 13.2) appear to linger for ~1s on close when linger is disabled.
-                // That causes delays when running the unit tests. Explicitly enableing linger but with a timeout of
+                // That causes delays when running the unit tests. Explicitly enabling linger but with a timeout of
                 // zero seconds seems to fix the issue.
                 s.setSoLinger(true, 0);
                 if (getLog().isTraceEnabled()) {
@@ -1216,7 +1216,7 @@ public abstract class AbstractEndpoint<S,U> {
                     getLog().trace("Socket unlock completed for:" + unlockAddress);
                 }
             }
-            // Wait for up to 1000ms acceptor threads to unlock. Particularly
+            // Wait for up to 1000ms for acceptor thread to unlock. Particularly
             // for the unit tests, we want to exit this loop as quickly as
             // possible. However, we also don't want to trigger excessive CPU
             // usage if the unlock takes longer than expected. Therefore, we


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org