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 cb7981933a95a4da8661e882eb9a98061952412b Author: opencode <[email protected]> AuthorDate: Thu Oct 15 12:36:30 2026 +0200 Shut down the utility executor created by an endpoint When no utility executor was set, AbstractEndpoint.getUtilityExecutor() created one lazily. The executor was never shut down when the endpoint was destroyed, so the non-daemon core thread of the scheduled thread pool it created kept running and prevented JVM shutdown, which is reachable for embedded users that do not provide the shared executor a full server installs from StandardServer. Track an executor created by the endpoint and shut it down in destroy(). Externally provided executors remain owned by the caller and are left running. The field is made volatile so the creation is safely published. Concurrent lazy creation remains an unsynchronised check-then-act: the executor is only requested from the protocol lifecycle start and from the scheduled async timeout task, which are serialised per endpoint, the same tolerated pattern already used for the connection latch in this class, and a duplicated pool in the hypothetical concurrent case would at worst match the previous behaviour since callers schedule tasks on the instance they received. The executor reference is deliberately not cleared on destroy so a later call returns the shut-down instance rather than creating a new pool that would never be destroyed. --- .../apache/tomcat/util/net/AbstractEndpoint.java | 33 +++++++++-- .../tomcat/util/net/TestAbstractEndpoint.java | 68 ++++++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java index 043474fcc4..71a15d6ba9 100644 --- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java +++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java @@ -1124,9 +1124,19 @@ public abstract class AbstractEndpoint<S, U> { /** - * External Executor based thread pool for utility tasks. + * External Executor based thread pool for utility tasks. Lazily created + * when none was set. The race between concurrent creators is benign: the + * executor is only requested from the protocol lifecycle and the + * scheduled async-timeout task, both serialised per endpoint, and a + * duplicated pool would at worst replicate the pre-existing behaviour. */ - private ScheduledExecutorService utilityExecutor = null; + private volatile ScheduledExecutorService utilityExecutor = null; + + /** + * The utility executor created by this endpoint, if any. It is tracked so + * that it can be shut down when the endpoint is destroyed. + */ + private volatile ScheduledExecutorService createdUtilityExecutor = null; /** * Set the utility executor. @@ -1138,16 +1148,19 @@ public abstract class AbstractEndpoint<S, U> { } /** - * Get the utility executor. + * Get the utility executor, creating one if none has been set. * * @return The utility executor */ public ScheduledExecutorService getUtilityExecutor() { - if (utilityExecutor == null) { + ScheduledExecutorService result = utilityExecutor; + if (result == null) { getLog().warn(sm.getString("endpoint.warn.noUtilityExecutor")); - utilityExecutor = new ScheduledThreadPoolExecutor(1); + result = new ScheduledThreadPoolExecutor(1); + utilityExecutor = result; + createdUtilityExecutor = result; } - return utilityExecutor; + return result; } @@ -2359,6 +2372,14 @@ public abstract class AbstractEndpoint<S, U> { * @throws java.lang.Exception If an error occurs while destroying the endpoint */ public final void destroy() throws Exception { + ScheduledExecutorService created = createdUtilityExecutor; + if (created != null) { + createdUtilityExecutor = null; + created.shutdown(); + } + // Deliberately leave utilityExecutor set: later calls to + // getUtilityExecutor() then return the shut-down executor instead of + // creating a new pool that would never be destroyed if (bindState == BindState.BOUND_ON_INIT) { unbind(); bindState = BindState.UNBOUND; diff --git a/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java b/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java new file mode 100644 index 0000000000..055cab7512 --- /dev/null +++ b/test/org/apache/tomcat/util/net/TestAbstractEndpoint.java @@ -0,0 +1,68 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.net; + +import java.util.concurrent.Executors; +import java.util.concurrent.ScheduledExecutorService; + +import org.junit.Assert; +import org.junit.Test; + +public class TestAbstractEndpoint { + + @Test + public void testUtilityExecutorIsCreatedOnce() throws Exception { + NioEndpoint endpoint = new NioEndpoint(); + try { + ScheduledExecutorService executor = endpoint.getUtilityExecutor(); + Assert.assertNotNull(executor); + Assert.assertSame(executor, endpoint.getUtilityExecutor()); + } finally { + endpoint.destroy(); + } + } + + @Test + public void testCreatedUtilityExecutorIsShutDownOnDestroy() throws Exception { + NioEndpoint endpoint = new NioEndpoint(); + ScheduledExecutorService executor = endpoint.getUtilityExecutor(); + Assert.assertFalse(executor.isShutdown()); + + endpoint.destroy(); + + Assert.assertTrue(executor.isShutdown()); + // A later request must not resurrect an unmanaged pool + Assert.assertSame(executor, endpoint.getUtilityExecutor()); + } + + @Test + public void testExternalUtilityExecutorIsNotShutDownOnDestroy() throws Exception { + NioEndpoint endpoint = new NioEndpoint(); + ScheduledExecutorService executor = Executors.newSingleThreadScheduledExecutor(); + endpoint.setUtilityExecutor(executor); + try { + Assert.assertSame(executor, endpoint.getUtilityExecutor()); + + endpoint.destroy(); + + // Externally provided executors are owned by the caller + Assert.assertFalse(executor.isShutdown()); + } finally { + executor.shutdown(); + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
