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 d58744f11d870f6674f30b87ded5cfff3f1181ea Author: opencode <[email protected]> AuthorDate: Mon Oct 12 23:02:09 2026 +0200 Synchronize access to the async timeout scheduling state The timeoutFuture and monitorFuture fields of AbstractProtocol were plain fields read and written from different threads without any synchronization or volatile semantics. monitorFuture is written by start() and cancelled and nulled by stop(), which may run on different threads. timeoutFuture is rescheduled by startAsyncTimeout(), running on a utility executor thread, while stopAsyncTimeout(), running on the thread calling stop(), cancels and nulls it. With the multi-threaded utility executor that the connector receives from the server, the check-then-act in startAsyncTimeout() and the cancel-then-null in stopAsyncTimeout() are racy. Guard both methods with a dedicated lock. All access to timeoutFuture now happens under that lock, so the field does not need to be volatile. monitorFuture is only accessed from the lifecycle methods so making it volatile is sufficient to establish visibility across threads. Also catch CancellationException when retrieving the result of a completed timeout task. A cancelled future reports isDone() as true and get() then throws CancellationException. Since that was not caught, the exception could propagate out of the monitor task and suppress all subsequent executions of the task, silently leaving asynchronous processing without any timeout. The exception is now logged and the timeout task is rescheduled. Note that a monitor execution already in progress when stop() runs can still re-arm the repeating timeout task once stop() has released the lock. The resulting task iterates the drained waiting processors set every second and is reclaimed by the next start/stop cycle or when the server executor shuts down. Closing that window fully would require an explicit stopped flag checked under the lock and may be addressed separately. --- java/org/apache/coyote/AbstractProtocol.java | 46 ++++++++++++++++------------ 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java index 5e96689c06..6236a78379 100644 --- a/java/org/apache/coyote/AbstractProtocol.java +++ b/java/org/apache/coyote/AbstractProtocol.java @@ -21,6 +21,7 @@ import java.net.InetAddress; import java.net.SocketException; import java.nio.ByteBuffer; import java.util.Set; +import java.util.concurrent.CancellationException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; @@ -90,12 +91,13 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, MBeanRegis /** Set of processors currently waiting for async processing. */ private final Set<Processor> waitingProcessors = ConcurrentHashMap.newKeySet(); - /** - * Controller for the timeout scheduling. - */ + /** Controller for the timeout scheduling. Accessed under timeoutSchedulingLock. */ private ScheduledFuture<?> timeoutFuture = null; /** Scheduled future for the periodic monitor task. */ - private ScheduledFuture<?> monitorFuture; + private volatile ScheduledFuture<?> monitorFuture; + + /** Guards the scheduling of the asynchronous timeout task. */ + private final Object timeoutSchedulingLock = new Object(); /** * Creates a new protocol handler. @@ -985,21 +987,23 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, MBeanRegis * represent a timeout that is triggered independently of the socket read/write timeouts. */ protected void startAsyncTimeout() { - if (timeoutFuture == null || timeoutFuture.isDone()) { - if (timeoutFuture != null && timeoutFuture.isDone()) { - // There was an error executing the scheduled task, get it and log it - try { - timeoutFuture.get(); - } catch (InterruptedException | ExecutionException e) { - getLog().error(sm.getString("abstractProtocolHandler.asyncTimeoutError"), e); + synchronized (timeoutSchedulingLock) { + if (timeoutFuture == null || timeoutFuture.isDone()) { + if (timeoutFuture != null && timeoutFuture.isDone()) { + // There was an error executing the scheduled task, get it and log it + try { + timeoutFuture.get(); + } catch (InterruptedException | ExecutionException | CancellationException e) { + getLog().error(sm.getString("abstractProtocolHandler.asyncTimeoutError"), e); + } } + timeoutFuture = getUtilityExecutor().scheduleAtFixedRate(() -> { + long now = System.currentTimeMillis(); + for (Processor processor : waitingProcessors) { + processor.timeoutAsync(now); + } + }, 1, 1, TimeUnit.SECONDS); } - timeoutFuture = getUtilityExecutor().scheduleAtFixedRate(() -> { - long now = System.currentTimeMillis(); - for (Processor processor : waitingProcessors) { - processor.timeoutAsync(now); - } - }, 1, 1, TimeUnit.SECONDS); } } @@ -1007,9 +1011,11 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler, MBeanRegis * Stops the async timeout scheduler. */ protected void stopAsyncTimeout() { - if (timeoutFuture != null) { - timeoutFuture.cancel(false); - timeoutFuture = null; + synchronized (timeoutSchedulingLock) { + if (timeoutFuture != null) { + timeoutFuture.cancel(false); + timeoutFuture = null; + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
