This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fbe1f7fa8c7ffff2429493a6f367e02e36be20da Author: opencode <[email protected]> AuthorDate: Tue Oct 13 13:59:32 2026 +0200 Always de-register upgraded connection statistics when the container closes the connection For external upgrade handlers, UpgradeInfo was registered with UpgradeGroupInfo in the upgrade processor constructor and only de-registered in the processor's close(), which runs only if the application closes the WebConnection. Connections closed by the container never triggered the de-registration, so handlers that do not close the WebConnection themselves leaked a registered UpgradeInfo per connection, growing the group's connection set without limit and keeping the statistics permanently counted as active. Remove the final modifier from UpgradeProcessorBase.recycle() and de-register the statistics in the recycle() implementation of UpgradeProcessorExternal, which is reached on every path on which the container releases an upgrade processor. --- .../http11/upgrade/UpgradeProcessorBase.java | 5 +- .../http11/upgrade/UpgradeProcessorExternal.java | 11 ++++ .../apache/coyote/http11/upgrade/TestUpgrade.java | 68 ++++++++++++++++++++++ 3 files changed, 82 insertions(+), 2 deletions(-) diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java index b0ecddbc96..62e0ae1a99 100644 --- a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java +++ b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorBase.java @@ -67,8 +67,9 @@ public abstract class UpgradeProcessorBase extends AbstractProcessorLight implem @Override - public final void recycle() { - // Currently a NO-OP as upgrade processors are not recycled. + public void recycle() { + // NO-OP. Sub-classes override this to release resources when the + // processor is released by the container. } diff --git a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorExternal.java b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorExternal.java index 500d7eb2cc..4109e11fa7 100644 --- a/java/org/apache/coyote/http11/upgrade/UpgradeProcessorExternal.java +++ b/java/org/apache/coyote/http11/upgrade/UpgradeProcessorExternal.java @@ -84,6 +84,17 @@ public class UpgradeProcessorExternal extends UpgradeProcessorBase { } + @Override + public void recycle() { + // Ensure the connection statistics are always de-registered when the + // processor is released by the container. The close() method below + // de-registers the statistics as well but it is only triggered if the + // application closes the WebConnection, which an upgrade handler is + // not required to do. + upgradeInfo.setGroupInfo(null); + } + + // --------------------------------------------------- AutoCloseable methods @Override diff --git a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java index 5b3a8de92b..d3d8ee9ecb 100644 --- a/test/org/apache/coyote/http11/upgrade/TestUpgrade.java +++ b/test/org/apache/coyote/http11/upgrade/TestUpgrade.java @@ -25,9 +25,12 @@ import java.io.OutputStreamWriter; import java.io.PrintWriter; import java.io.Reader; import java.io.Writer; +import java.lang.reflect.Field; import java.net.Socket; import java.net.SocketException; import java.nio.charset.StandardCharsets; +import java.util.Set; +import java.util.function.BooleanSupplier; import javax.net.SocketFactory; @@ -50,6 +53,7 @@ import org.apache.catalina.Context; import org.apache.catalina.startup.Tomcat; import org.apache.catalina.startup.TomcatBaseTest; import org.apache.coyote.UpgradeToken; +import org.apache.coyote.http11.AbstractHttp11Protocol; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; import org.apache.tomcat.ContextBind; @@ -279,6 +283,50 @@ public class TestUpgrade extends TomcatBaseTest { Assert.assertNull(listener.error); } + @Test + public void testUpgradeInfoUnregisteredWhenConnectionClosed() throws Exception { + // The upgrade handler never closes the WebConnection so all clean-up + // has to be performed by the container when the connection is closed. + UpgradeConnection uc = doUpgrade(NoClose.class); + + AbstractHttp11Protocol<?> protocol = + (AbstractHttp11Protocol<?>) getTomcatInstance().getConnector().getProtocolHandler(); + UpgradeGroupInfo groupInfo = protocol.getUpgradeGroupInfo("test"); + + Set<UpgradeInfo> upgradeInfos = getUpgradeInfos(groupInfo); + waitForCondition(() -> upgradeInfos.size() == 1, 5000); + Assert.assertEquals(1, upgradeInfos.size()); + + // Closing the connector closes the connection. This is the only event + // that can trigger the clean-up of the upgrade registration. + getTomcatInstance().stop(); + + waitForCondition(upgradeInfos::isEmpty, 10000); + + // The registration must not outlive the connection. + Assert.assertEquals("The UpgradeInfo remained registered after the connection was closed.", 0, + upgradeInfos.size()); + + uc.shutdownInput(); + uc.shutdownOutput(); + } + + + @SuppressWarnings("unchecked") + private static Set<UpgradeInfo> getUpgradeInfos(UpgradeGroupInfo groupInfo) throws Exception { + Field upgradeInfosField = UpgradeGroupInfo.class.getDeclaredField("upgradeInfos"); + upgradeInfosField.setAccessible(true); + return (Set<UpgradeInfo>) upgradeInfosField.get(groupInfo); + } + + + private static void waitForCondition(BooleanSupplier condition, long timeoutMs) throws InterruptedException { + long deadline = System.currentTimeMillis() + timeoutMs; + while (!condition.getAsBoolean() && System.currentTimeMillis() < deadline) { + Thread.sleep(50); + } + } + private void doTestCheckClosed( Class<? extends HttpUpgradeHandler> upgradeHandlerClass) throws Exception { @@ -468,6 +516,26 @@ public class TestUpgrade extends TomcatBaseTest { } + public static class NoClose implements HttpUpgradeHandler { + @Override + public void init(WebConnection connection) { + // Open the streams to make the upgraded connection fully active but + // never close the WebConnection. + try { + connection.getInputStream(); + connection.getOutputStream(); + } catch (IOException ioe) { + throw new IllegalStateException(ioe); + } + } + + @Override + public void destroy() { + // NO-OP + } + } + + public static class EchoNonBlocking implements HttpUpgradeHandler { @Override --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
