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 18:53:57 UTC
(tomcat) branch 10.1.x updated (443e9be641 -> b76216af5d)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 443e9be641 getDeferAccept() always returns false - remove unused code
new d30a545b31 SocketTimeout has no impact for acceptor unlock - remove it
new b79a946cbc Allow any positive value for socket.unlockTimeout rather than >=2s
new ba98642273 Refactor to specify acceptor stop wait time in ms rather than s
new b76216af5d 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 | 15 ++++-----------
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(+), 20 deletions(-)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/04: SocketTimeout has no impact for acceptor unlock - remove it
Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit d30a545b319abfcdadc9f94d26a529324c9401b5
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Apr 19 17:45:56 2024 +0100
SocketTimeout has no impact for acceptor unlock - remove it
---
java/org/apache/tomcat/util/net/AbstractEndpoint.java | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 7bae5f68ab..a46ee4ce9e 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1129,15 +1129,12 @@ public abstract class AbstractEndpoint<S,U> {
unlockAddress = getUnlockAddress(localAddress);
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);
+ // Never going to read from this socket so the timeout doesn't matter. Use the unlock timeout.
+ s.setSoTimeout(utmo);
// 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
// zero seconds seems to fix the issue.
---------------------------------------------------------------------
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 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b79a946cbc3fa55073c733dbf4a9ab1c5fdbd003
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 | 8 ++------
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, 26 insertions(+), 9 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index a46ee4ce9e..460e7543f5 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1129,12 +1129,8 @@ public abstract class AbstractEndpoint<S,U> {
unlockAddress = getUnlockAddress(localAddress);
try (java.net.Socket s = new java.net.Socket()) {
- int utmo = 2 * 1000;
- if (getSocketProperties().getUnlockTimeout() > utmo) {
- utmo = getSocketProperties().getUnlockTimeout();
- }
// Never going to read from this socket so the timeout doesn't matter. Use the unlock timeout.
- s.setSoTimeout(utmo);
+ s.setSoTimeout(getSocketProperties().getUnlockTimeout());
// 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
// zero seconds seems to fix the issue.
@@ -1142,7 +1138,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 (getLog().isTraceEnabled()) {
getLog().trace("Socket unlock completed for:" + unlockAddress);
}
diff --git a/java/org/apache/tomcat/util/net/LocalStrings.properties b/java/org/apache/tomcat/util/net/LocalStrings.properties
index 8557463f7e..35c9d890c2 100644
--- a/java/org/apache/tomcat/util/net/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/LocalStrings.properties
@@ -139,6 +139,8 @@ sniExtractor.tooEarly=It is illegal to call this method before the client hello
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 b91d54f0e2..10b9fd765f 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 <Connector> element
* in server.xml. All properties are prefixed with "socket."
@@ -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
@@ -451,7 +458,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"));
+ }
}
/**
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 3a295c0937..53ebb4a103 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -153,6 +153,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 010f40372c..d11f51c893 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -823,8 +823,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 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ba98642273f1ce9a81857506dffb6dc370d7d2f9
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 460e7543f5..03a12d1959 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -1493,7 +1493,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 b1083b292a..d8deb8cb26 100644
--- a/java/org/apache/tomcat/util/net/Acceptor.java
+++ b/java/org/apache/tomcat/util/net/Acceptor.java
@@ -171,12 +171,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 5bbc3a11f8..0c3cbad675 100644
--- a/java/org/apache/tomcat/util/net/Nio2Endpoint.java
+++ b/java/org/apache/tomcat/util/net/Nio2Endpoint.java
@@ -199,7 +199,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(() -> {
@@ -437,11 +437,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 508682c8fc..c53164e256 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -296,7 +296,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 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b76216af5d8ad0608c3da5f0a9a273f3b46f647e
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 c53164e256..9f646e73eb 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -296,7 +296,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 53ebb4a103..c2a903f944 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -158,6 +158,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