Fixes to ConnectionLimit for use with WebSocket (#13350)

* Added method `AcceptListener.onClosed()`.
* Disabled `WebSocketOverHTTP2Test.testConnectionLimit()`.
* Refinements to `WebSocketConnection` OPEN state
* Introduced NetworkConnectionLimit and associated Jetty module.
* Deprecated ConnectionLimit and associated Jetty module, and removed their usages in favor of NetworkConnectionLimit.

Signed-off-by: Lachlan Roberts <[email protected]>
Signed-off-by: Simone Bordet <[email protected]>
Co-authored-by: Simone Bordet <[email protected]>

Author: lachlan-roberts

documentation/jetty/modules/code/examples/src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java

@@ -62,7 +62,6 @@
 import org.eclipse.jetty.rewrite.handler.RewriteHandler;
 import org.eclipse.jetty.rewrite.handler.RewriteRegexRule;
 import org.eclipse.jetty.server.ConnectionFactory;
-import org.eclipse.jetty.server.ConnectionLimit;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.CustomRequestLog;
 import org.eclipse.jetty.server.FormFields;
@@ -71,6 +70,7 @@
 import org.eclipse.jetty.server.HttpConnectionFactory;
 import org.eclipse.jetty.server.MemoryConnector;
 import org.eclipse.jetty.server.MemoryTransport;
+import org.eclipse.jetty.server.NetworkConnectionLimit;
 import org.eclipse.jetty.server.NetworkConnector;
 import org.eclipse.jetty.server.ProxyConnectionFactory;
 import org.eclipse.jetty.server.Request;
@@ -357,14 +357,14 @@ public void onOpen(NetworkConnector connector)
         // end::sameRandomPort[]
     }
 
-    public void connectionLimit()
+    public void networkConnectionLimit()
     {
-        // tag::connectionLimit[]
+        // tag::networkConnectionLimit[]
         Server server = new Server();
 
-        // Limit connections to the server, across all connectors.
-        ConnectionLimit serverConnectionLimit = new ConnectionLimit(1024, server);
-        server.addBean(serverConnectionLimit);
+        // Limit TCP connections to the server, across all connectors.
+        NetworkConnectionLimit serverNetworkConnectionLimit = new NetworkConnectionLimit(1024, server);
+        server.addBean(serverNetworkConnectionLimit);
 
         ServerConnector connector1 = new ServerConnector(server);
         connector1.setPort(8080);
@@ -374,9 +374,9 @@ public void connectionLimit()
         connector2.setPort(9090);
         server.addConnector(connector2);
         // Limit connections for this connector only.
-        ConnectionLimit connectorConnectionLimit = new ConnectionLimit(64, connector2);
-        connector2.addBean(connectorConnectionLimit);
-        // end::connectionLimit[]
+        NetworkConnectionLimit connectorNetworkConnectionLimit = new NetworkConnectionLimit(64, connector2);
+        connector2.addBean(connectorNetworkConnectionLimit);
+        // end::networkConnectionLimit[]
     }
 
     public void sslHandshakeListener() throws Exception

documentation/jetty/modules/operations-guide/pages/modules/standard.adoc

@@ -36,7 +36,7 @@ Existing, established connections will work normally.
 
 Once accepting new connections has been suspended, Jetty will periodically check whether the rate has returned withing the limits, and if so, accepting new connections will be resumed.
 
-This module is often used in conjunction with the xref:connection-limit[`connection-limit`] module.
+This module is often used in conjunction with the xref:network-connection-limit[`network-connection-limit`] module.
 
 NOTE: The number of connections seen at the JVM level may be different from the number of connections seen at the OS level.
 For more information, refer to xref:programming-guide:server/http.adoc#connector-limiting[this section].
@@ -75,24 +75,24 @@ This property allows you to cap the max heap memory retained by the pool.
 `jetty.byteBufferPool.maxDirectMemory`::
 This property allows you to cap the max direct memory retained by the pool.
 
-[[connection-limit]]
-== Module `connection-limit`
+[[network-connection-limit]]
+== Module `network-connection-limit`
 
-The `connection-limit` module limits the number of connections accepted by the server, across all connectors.
+The `network-connection-limit` module limits the number of network connections accepted by the server, across all connectors.
 
-Once the configured maximum number of connections is reached, Jetty will not accept more connections.
-Existing, established connections will work normally.
-When existing connections are closed, accepting new connections will be resumed.
+Once the configured maximum number of network connections is reached, Jetty will not accept more network connections.
+Existing, established network connections will work normally.
+When existing network connections are closed, accepting new network connections will be resumed.
 
 This module is often used in conjunction with the xref:accept-rate-limit[`accept-rate-limit`] module.
 
-NOTE: The number of connections seen at the JVM level may be different from the number of connections seen at the OS level.
+NOTE: The number of network connections seen at the JVM level may be different from the number of network connections seen at the OS level.
 For more information, refer to xref:programming-guide:server/http.adoc#connector-limiting[this section].
 
-The module file is `$JETTY_HOME/modules/connection-limit.mod`:
+The module file is `$JETTY_HOME/modules/network-connection-limit.mod`:
 
 ----
-include::{jetty-home}/modules/connection-limit.mod[tags=documentation]
+include::{jetty-home}/modules/network-connection-limit-limit.mod[tags=documentation]
 ----
 
 [[console-capture]]

documentation/jetty/modules/programming-guide/pages/arch/io.adoc

@@ -132,10 +132,7 @@ This is useful in many cases, for example:
 `Connection.Listener` implementations must be added as beans to a server-side `Connector`, or to client-side `HttpClient`, `WebSocketClient`, `HTTP2Client` or `HTTP3Client`.
 You can add as beans many `Connection.Listener` objects, each with its own logic, so that you can separate different logics into different `Connection.Listener` implementations.
 
-The Jetty I/O library provides useful `Connection.Listener` implementations that you should evaluate before writing your own:
-
-* link:{javadoc-url}/org/eclipse/jetty/io/ConnectionStatistics.html[`ConnectionStatistics`]
-* link:{javadoc-url}/org/eclipse/jetty/server/ConnectionLimit.html[`ConnectionLimit`]
+The Jetty I/O library provides useful `Connection.Listener` implementations that you should evaluate before writing your own, for example link:{javadoc-url}/org/eclipse/jetty/io/ConnectionStatistics.html[`ConnectionStatistics`].
 
 Here is a simple example of a `Connection.Listener` used both on the client and on the server:
 

documentation/jetty/modules/programming-guide/pages/server/http.adoc

@@ -404,22 +404,22 @@ include::code:example$src/main/java/org/eclipse/jetty/docs/programming/server/ht
 [[connector-limiting]]
 === Limiting Connections
 
-It is possible to limit the number of connections accepted by the whole server (and therefore across all connectors), or by a specific connector.
+It is possible to limit the number of TCP connections accepted by the whole server (and therefore across all connectors), or by a specific connector.
 
-This feature is implemented by class `org.eclipse.jetty.server.ConnectionLimit` and you can use it in this way:
+This feature is implemented by class `org.eclipse.jetty.server.NetworkConnectionLimit` and you can use it in this way:
 
 [,java,indent=0,options=nowrap]
 ----
-include::code:example$src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=connectionLimit]
+include::code:example$src/main/java/org/eclipse/jetty/docs/programming/server/http/HTTPServerDocs.java[tags=networkConnectionLimit]
 ----
 
 [NOTE]
 ====
-When the maximum number of connections is reached, no more connections will be accepted _at the JVM level_ -- but they could be accepted at the OS level.
+When the maximum number of TCP connections is reached, no more TCP connections will be accepted _at the JVM level_ -- but they could be accepted at the OS level.
 
-This means that if you are using OS tools (like Linux's `ss`) to count the number of established connections, you may find a number that may be greater than the maximum number of connections configured in a `ConnectionLimit`.
+This means that if you are using OS tools (like Linux's `ss`) to count the number of established TCP connections, you may find a number that may be greater than the maximum number of TCP connections configured in a `NetworkConnectionLimit`.
 
-Note also that different operative systems may behave differently when Jetty is not accepting connections: some OS accepts connections at the TCP level anyway (but does not notify this event to the JVM), some other OS may not accept connections at the TCP level.
+Note also that different operative systems may behave differently when Jetty is not accepting TCP connections: some OS accepts connections at the TCP level anyway (but does not notify this event to the JVM), some other OS may not accept connections at the TCP level.
 ====
 
 [[connector-protocol]]

jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2StreamEndPoint.java

@@ -183,6 +183,7 @@ public void close(Throwable cause)
                 data.release();
             shutdownOutput();
             stream.close();
+            connection.onClose(cause);
             onClose(cause);
         }
     }

jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/SelectorManager.java

@@ -309,6 +309,21 @@ protected void endPointOpened(EndPoint endpoint)
      */
     protected void endPointClosed(EndPoint endpoint)
     {
+        Object transport = endpoint.getTransport();
+        if (transport instanceof SelectableChannel selectableChannel)
+        {
+            for (AcceptListener l : _acceptListeners)
+            {
+                try
+                {
+                    l.onClosed(selectableChannel);
+                }
+                catch (Throwable x)
+                {
+                    LOG.warn("Failed to notify onClosed() on listener {}", l, x);
+                }
+            }
+        }
     }
 
     /**
@@ -456,7 +471,7 @@ protected void onAccepting(SelectableChannel channel)
             }
             catch (Throwable x)
             {
-                LOG.warn("Failed to notify onAccepting on listener {}", l, x);
+                LOG.warn("Failed to notify onAccepting() on listener {}", l, x);
             }
         }
     }
@@ -471,7 +486,7 @@ protected void onAcceptFailed(SelectableChannel channel, Throwable cause)
             }
             catch (Throwable x)
             {
-                LOG.warn("Failed to notify onAcceptFailed on listener {}", l, x);
+                LOG.warn("Failed to notify onAcceptFailed() on listener {}", l, x);
             }
         }
     }
@@ -486,7 +501,7 @@ protected void onAccepted(SelectableChannel channel)
             }
             catch (Throwable x)
             {
-                LOG.warn("Failed to notify onAccepted on listener {}", l, x);
+                LOG.warn("Failed to notify onAccepted() on listener {}", l, x);
             }
         }
     }
@@ -533,6 +548,15 @@ default void onAcceptFailed(SelectableChannel channel, Throwable cause)
         default void onAccepted(SelectableChannel channel)
         {
         }
+
+        /**
+         * Called when an accepted {@link SelectableChannel} is closed.
+         *
+         * @param channel the accepted channel
+         */
+        default void onClosed(SelectableChannel channel)
+        {
+        }
     }
 
     @Override

jetty-core/jetty-server/src/main/config/etc/jetty-network-connection-limit.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0"?>
+<!DOCTYPE Configure PUBLIC "-//Jetty//Configure//EN" "https://jetty.org/configure_10_0.dtd">
+<Configure id="Server" class="org.eclipse.jetty.server.Server">
+  <Call name="addBean">
+    <Arg>
+      <New class="org.eclipse.jetty.server.NetworkConnectionLimit">
+        <Arg name= "maxNetworkConnectionCount" type="int">
+          <Property name="jetty.networkConnectionLimit.maxNetworkConnectionCount" default="1000" />
+        </Arg>
+        <Arg name="server">
+          <Ref refid="Server" />
+        </Arg>
+        <Set name="endPointIdleTimeout"><Property name="jetty.networkConnectionLimit.endPointIdleTimeout" default="1000" /></Set>
+      </New>
+    </Arg>
+  </Call>
+</Configure>

jetty-core/jetty-server/src/main/config/modules/connection-limit.mod

@@ -5,6 +5,10 @@ Enables a server-wide limit on TCP connections.
 
 [tags]
 connector
+deprecated
+
+[deprecated]
+Use module network-connection-limit instead.
 
 [depend]
 server

jetty-core/jetty-server/src/main/config/modules/network-connection-limit.mod

@@ -0,0 +1,20 @@
+[description]
+Enables a server-wide limit on the number of connected EndPoints.
+
+[tags]
+connector
+
+[depend]
+server
+
+[xml]
+etc/jetty-network-connection-limit.xml
+
+[ini-template]
+#tag::documentation[]
+## The maximum number of network connections allowed across all connectors.
+#jetty.networkConnectionLimit.maxNetworkConnectionCount=1000
+
+## The idle timeout to apply (in milliseconds) to existing EndPoints when the limit is reached.
+#jetty.networkConnectionLimit.endPointIdleTimeout=1000
+#end::documentation[]

jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ConnectionLimit.java

@@ -56,7 +56,9 @@
  * @see LowResourceMonitor
  * @see Connection.Listener
  * @see SelectorManager.AcceptListener
+ * @deprecated use {@link NetworkConnectionLimit} instead
  */
+@Deprecated(forRemoval = true, since = "12.1.0")
 @ManagedObject
 public class ConnectionLimit extends AbstractLifeCycle implements Listener, SelectorManager.AcceptListener
 {