You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by re...@apache.org on 2026/10/12 07:46:48 UTC
(tomcat) branch 9.0.x updated (07503a0814 -> 77e5a92982)
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a change to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
from 07503a0814 Follow-up to "Report malformed JNDIRealm user patterns..."
new ff99ddd6db Wake up pool waiters on stop and never leak receive tasks
new 87e233ece6 Cap the NIO sender SO_TIMEOUT at Integer.MAX_VALUE
new 3d3cd68605 Do not poll or swallow interrupts when waiting for a pooled sender
new 9bc3b9c629 Apply the configured UDP buffer sizes of a cluster sender
new e7ac3bfa56 Fail the receiver start when the cluster bind address cannot be resolved
new df2ba6d608 Stop the listener thread from reviving the listening flag of a stopped receiver
new 04bfebd4f2 Release the opened channels and selector when a cluster receiver bind fails
new b4d7350231 Report a class name filter match failure via the documented exception when no Log is configured
new dc25b0914e Fix a data race in the lazy initialisation of the auto-generated rate limit policy name
new fe8a1dfa49 Harden RateLimiterBase.destroy() against a limiter whose configuration failed
new 7184caaffa Reject bucket durations that overflow the shift based time bucket counter
new 4fe0dff2bc Stop truncating the time bucket index to an int in TimeBucketCounterBase
new 6aca9be31d Guard the time bucket counter maintenance futures against a racing destroy()
new 77e5a92982 Escape literal single quotes in Strftime SimpleDateFormat patterns
The 14 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:
.../catalina/tribes/transport/AbstractSender.java | 2 +
.../tribes/transport/LocalStrings.properties | 1 +
.../catalina/tribes/transport/PooledSender.java | 42 ++--
.../catalina/tribes/transport/ReceiverBase.java | 31 ++-
.../catalina/tribes/transport/RxTaskPool.java | 12 +-
.../catalina/tribes/transport/nio/NioReceiver.java | 226 +++++++++++++--------
.../tribes/transport/nio/NioReplicationTask.java | 102 +++++-----
.../catalina/tribes/transport/nio/NioSender.java | 10 +-
.../catalina/util/CustomObjectInputStream.java | 3 +-
.../apache/catalina/util/LocalStrings.properties | 1 +
java/org/apache/catalina/util/RateLimiterBase.java | 24 ++-
java/org/apache/catalina/util/Strftime.java | 32 +--
.../apache/catalina/util/TimeBucketCounter.java | 22 ++
.../catalina/util/TimeBucketCounterBase.java | 63 ++++--
.../tribes/transport/TestAbstractSender.java | 96 +++++++++
.../tribes/transport/TestReceiverBase.java | 129 ++++++++++++
.../tribes/transport/nio/TestNioReceiver.java | 199 ++++++++++++++++++
test/org/apache/catalina/util/TestStrftime.java | 72 +++++++
.../catalina/util/TestTimeBucketCounter.java | 30 ++-
webapps/docs/changelog.xml | 17 ++
20 files changed, 915 insertions(+), 199 deletions(-)
create mode 100644 test/org/apache/catalina/tribes/transport/TestAbstractSender.java
create mode 100644 test/org/apache/catalina/tribes/transport/TestReceiverBase.java
create mode 100644 test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
create mode 100644 test/org/apache/catalina/util/TestStrftime.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 12/14: Stop truncating the time bucket index to an int in TimeBucketCounterBase
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 4fe0dff2bc11e07736d9a5c947b61310d3d0f7e4
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 22:24:19 2026 +0200
Stop truncating the time bucket index to an int in TimeBucketCounterBase
getCurrentBucketPrefix() narrowed the long value returned by
getBucketIndex() to an int. periodicEvict() builds the current and
next bucket prefixes from that value to decide which keys survive.
Bucket keys are generated from the full long index, so once the
index exceeds Integer.MAX_VALUE the truncated prefix no longer
matches any key, every entry is evicted on each maintenance run and
the RateLimitFilter silently stops enforcing limits.
With one-second buckets the shift based TimeBucketCounter reaches
that point when System.currentTimeMillis() hits 2^41 ms, i.e. in
2039, and the division based ExactTimeBucketCounter even earlier, in
2038. Buckets of a minute or more overflow only millennia out, which
is why the bug survived unnoticed.
Return the full long index from getCurrentBucketPrefix(). This
widens the return type of a public method, but the method is part of
the recently added rate limiting support classes, has no callers
outside TimeBucketCounterBase and its own unit test, and is not
referenced in the documentation. The only production caller,
periodicEvict(), already stored the result in a long local, and the
remaining int casts in TimeBucketCounter operate on durations, which
are range-checked separately. Widen the two test locals to match; a
regression test for the pre-2038 truncation would require injecting a
fake clock, and re-narrowing is now prevented by the compiler.
---
java/org/apache/catalina/util/TimeBucketCounterBase.java | 4 ++--
test/org/apache/catalina/util/TestTimeBucketCounter.java | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/java/org/apache/catalina/util/TimeBucketCounterBase.java b/java/org/apache/catalina/util/TimeBucketCounterBase.java
index 01a51b1ec0..7aec8de153 100644
--- a/java/org/apache/catalina/util/TimeBucketCounterBase.java
+++ b/java/org/apache/catalina/util/TimeBucketCounterBase.java
@@ -151,8 +151,8 @@ public abstract class TimeBucketCounterBase {
*
* @return bucket index
*/
- public int getCurrentBucketPrefix() {
- return (int) getBucketIndex(System.currentTimeMillis());
+ public long getCurrentBucketPrefix() {
+ return getBucketIndex(System.currentTimeMillis());
}
diff --git a/test/org/apache/catalina/util/TestTimeBucketCounter.java b/test/org/apache/catalina/util/TestTimeBucketCounter.java
index 4a2d66c996..ed385a900f 100644
--- a/test/org/apache/catalina/util/TestTimeBucketCounter.java
+++ b/test/org/apache/catalina/util/TestTimeBucketCounter.java
@@ -80,7 +80,7 @@ public class TestTimeBucketCounter {
@Test
public void testGetMillisUntilNextBucket() throws InterruptedException {
long millis;
- int tb1, tb2;
+ long tb1, tb2;
TimeBucketCounter tbc = new TimeBucketCounter(2, new java.util.concurrent.ScheduledThreadPoolExecutor(1));
tb1 = tbc.getCurrentBucketPrefix();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 03/14: Do not poll or swallow interrupts when waiting for a pooled sender
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 3d3cd686050d2e478190cbec46cdf0a5765999a7
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 16:10:01 2026 +0200
Do not poll or swallow interrupts when waiting for a pooled sender
SenderQueue.getSender() parks on the queue monitor until a sender becomes
available. Two problems made the wait misbehave as soon as maxWait was
zero or negative:
- The expiry test was "delta > timeout && timeout > 0", which can never
succeed for a non-positive timeout, and the wait amount was then forced
to 1 ms. The result was an endless loop waking every millisecond:
continuous CPU churn while a blocked cluster send could not be woken by
anything other than a sender eventually being returned.
- The InterruptedException was ignored without restoring the interrupt
status, so interrupting such a thread had no effect at all. It would
only exit later, when the queue was closed, and then via an unchecked
IllegalStateException escaping the caller.
Wait once for the full remaining timeout when a positive timeout is
configured, and wait without a timeout when it is not. No polling is
needed: returnSender(), open() and close() all notify every waiting
thread under the same monitor. On interruption, restore the interrupt
status and return null, which the single caller, PooledParallelSender
.sendMessage(), already reports as a ChannelException for the destination
members. Document the non-positive maxWait semantics on getSender() and
setMaxWait(); this preserves the observable behaviour, as a non-positive
timeout already blocked indefinitely before.
Since the wait no longer re-checks periodically, make the pool limit a
notifier as well: setLimit() is now synchronized and wakes the waiting
threads. Raising the limit can make room for a new sender, and previously
only the 1 ms polling happened to notice it. Synchronizing the setter also
removes an unsynchronized write of the limit, which getSender() and
returnSender() read under the monitor.
---
.../catalina/tribes/transport/PooledSender.java | 42 +++++++++++++++-------
1 file changed, 30 insertions(+), 12 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/PooledSender.java b/java/org/apache/catalina/tribes/transport/PooledSender.java
index 80e1d961a2..c4b46a451a 100644
--- a/java/org/apache/catalina/tribes/transport/PooledSender.java
+++ b/java/org/apache/catalina/tribes/transport/PooledSender.java
@@ -52,8 +52,12 @@ public abstract class PooledSender extends AbstractSender implements MultiPointS
public abstract DataSender getNewDataSender();
/**
- * Retrieves a sender from the pool, waiting up to maxWait milliseconds.
- * @return a DataSender from the pool, or null if timeout expires
+ * Retrieves a sender from the pool, waiting up to maxWait milliseconds. If maxWait is zero or negative, the call
+ * waits indefinitely for a sender to become available.
+ *
+ * @return a DataSender from the pool, or null if the wait timed out or the calling thread was interrupted
+ *
+ * @throws IllegalStateException if the sender pool has been closed
*/
public DataSender getSender() {
return queue.getSender(getMaxWait());
@@ -124,7 +128,9 @@ public abstract class PooledSender extends AbstractSender implements MultiPointS
}
/**
- * Sets the maximum time to wait for a sender from the pool, in milliseconds.
+ * Sets the maximum time to wait for a sender from the pool, in milliseconds. A value of zero or less means the
+ * wait for a sender is unbounded.
+ *
* @param maxWait the maximum wait time
*/
public void setMaxWait(long maxWait) {
@@ -172,15 +178,17 @@ public abstract class PooledSender extends AbstractSender implements MultiPointS
/**
* @return Returns the limit.
*/
- public int getLimit() {
+ public synchronized int getLimit() {
return limit;
}
/**
* @param limit The limit to set.
*/
- public void setLimit(int limit) {
+ public synchronized void setLimit(int limit) {
this.limit = limit;
+ // Raising the limit can make room for a new sender to be created
+ notifyAll();
}
public synchronized int getInUsePoolSize() {
@@ -217,15 +225,25 @@ public abstract class PooledSender extends AbstractSender implements MultiPointS
inuse.add(sender);
return sender;
}
- long delta = System.currentTimeMillis() - start;
- if (delta > timeout && timeout > 0) {
- return null;
- }
- long remaining = timeout > 0 ? Math.max(timeout - delta, 1) : 1;
try {
- wait(remaining);
+ if (timeout > 0) {
+ long delta = System.currentTimeMillis() - start;
+ if (delta >= timeout) {
+ return null;
+ }
+ // Everything that can make a sender available - returnSender(),
+ // open(), close() and setLimit() - notifies every waiting
+ // thread, so there is no need to poll.
+ wait(timeout - delta);
+ } else {
+ // No timeout configured: wait until a sender becomes
+ // available or the queue is closed.
+ wait();
+ }
} catch (InterruptedException x) {
- // Ignore
+ // Restore the interrupt status and let the caller handle it
+ Thread.currentThread().interrupt();
+ return null;
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 11/14: Reject bucket durations that overflow the shift based time bucket counter
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 7184caaffa59e925cb42a9cd96c0b7aac04d0a0d
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 22:13:23 2026 +0200
Reject bucket durations that overflow the shift based time bucket counter
TimeBucketCounter computes its bucket size as duration * 1000 in int
arithmetic and rounds it up to the next power of 2. For durations in
the range above 1073741 seconds the product exceeds 2^30,
nextPowerOf2 returns Integer.MIN_VALUE, the bit-count loop yields
zero bits and the base class constructor rejected the resulting zero
duration with the misleading message "The duration (in seconds) must
be a positive integer, was [0]". Worse, for durations of 2147484
seconds and above the millisecond product wraps to a small positive
value: the counter was silently constructed and enforced a one or two
second bucket instead of the configured multi-week window, rejecting
legitimate traffic.
Reject durations larger than 1073741 seconds, the largest value for
which the actual duration in milliseconds, rounded up to the next
power of 2, still fits in an int, with a dedicated error message. The
check is placed in determineShiftBitsOfDuration() so that it runs
before the super() constructor argument and the ratio computation.
Configurations that need longer windows have always been able to
select ExactRateLimiter, whose counter divides exactly and is not
limited by the shift arithmetic.
Add unit tests covering the maximum accepted duration, a duration
just above the maximum and one that previously passed silently due to
wrap-around.
---
.../apache/catalina/util/LocalStrings.properties | 1 +
.../apache/catalina/util/TimeBucketCounter.java | 22 +++++++++++++++++
.../catalina/util/TestTimeBucketCounter.java | 28 ++++++++++++++++++++++
3 files changed, 51 insertions(+)
diff --git a/java/org/apache/catalina/util/LocalStrings.properties b/java/org/apache/catalina/util/LocalStrings.properties
index b418e33225..7f3a337f4f 100644
--- a/java/org/apache/catalina/util/LocalStrings.properties
+++ b/java/org/apache/catalina/util/LocalStrings.properties
@@ -58,5 +58,6 @@ sessionIdGeneratorBase.random=Exception initializing random number generator of
sessionIdGeneratorBase.randomAlgorithm=Exception initializing random number generator using algorithm [{0}]
sessionIdGeneratorBase.randomProvider=Exception initializing random number generator using provider [{0}]
+timebucket.durationTooLarge=The duration (in seconds) must not exceed [{1}], was [{0}]
timebucket.invalidDuration=The duration (in seconds) must be a positive integer, was [{0}]
timebucket.maintenance.error=Error processing periodic maintenance
diff --git a/java/org/apache/catalina/util/TimeBucketCounter.java b/java/org/apache/catalina/util/TimeBucketCounter.java
index 4f54ade92f..4282840d83 100644
--- a/java/org/apache/catalina/util/TimeBucketCounter.java
+++ b/java/org/apache/catalina/util/TimeBucketCounter.java
@@ -18,11 +18,20 @@ package org.apache.catalina.util;
import java.util.concurrent.ScheduledExecutorService;
+import org.apache.tomcat.util.res.StringManager;
+
/**
* A fast counter that optimizes efficiency at the expense of approximate bucket indexing.
*/
public class TimeBucketCounter extends TimeBucketCounterBase {
+ private static final StringManager sm = StringManager.getManager(TimeBucketCounter.class);
+
+ // The largest configured duration, in seconds, for which the actual duration in milliseconds rounded up to the
+ // next power of 2 still fits in an int, as required by the int based bit shift arithmetic of this
+ // implementation. Longer durations should use ExactTimeBucketCounter, which divides exactly.
+ private static final int MAX_DURATION_SECONDS = (int) ((1L << 30) / 1000);
+
// Milliseconds bucket size as a Power of 2 for bit shift math, e.g. 16 for 65_536ms which is about 1:05 minute.
private final int numBits;
@@ -35,6 +44,10 @@ public class TimeBucketCounter extends TimeBucketCounterBase {
*
* @param bucketDuration the bucket duration in seconds
* @param executorService the executor service for maintenance tasks
+ *
+ * @throws IllegalArgumentException if bucketDuration is not positive, or is larger than the maximum supported
+ * duration of the shift based arithmetic (see
+ * {@link #determineShiftBitsOfDuration(int)})
*/
public TimeBucketCounter(int bucketDuration, ScheduledExecutorService executorService) {
super(getActualDuration(bucketDuration), executorService);
@@ -83,8 +96,17 @@ public class TimeBucketCounter extends TimeBucketCounterBase {
* @param duration bucket duration in seconds
*
* @return bits to be shifted
+ *
+ * @throws IllegalArgumentException if the duration in milliseconds, rounded up to the next power of 2, would not
+ * fit in an int
*/
protected static int determineShiftBitsOfDuration(int duration) {
+ if (duration > MAX_DURATION_SECONDS) {
+ // duration * 1000, rounded up to the next power of 2, would overflow int. Reject it with a clear
+ // message rather than computing a meaningless number of bits.
+ throw new IllegalArgumentException(sm.getString("timebucket.durationTooLarge", Integer.valueOf(duration),
+ Integer.valueOf(MAX_DURATION_SECONDS)));
+ }
int bits = 0;
int pof2 = nextPowerOf2(duration * 1000);
int bitCheck = pof2;
diff --git a/test/org/apache/catalina/util/TestTimeBucketCounter.java b/test/org/apache/catalina/util/TestTimeBucketCounter.java
index 43f505eb43..4a2d66c996 100644
--- a/test/org/apache/catalina/util/TestTimeBucketCounter.java
+++ b/test/org/apache/catalina/util/TestTimeBucketCounter.java
@@ -49,6 +49,34 @@ public class TestTimeBucketCounter {
Assert.assertEquals(1.092, tbc.getRatio(), DELTA);
}
+ @Test
+ public void testDurationAtMaximum() {
+ // 1073741 seconds is the largest duration whose millisecond form, rounded up to the next power of 2
+ // (2^30), still fits in an int
+ TimeBucketCounter tbc = new TimeBucketCounter(1073741,
+ new java.util.concurrent.ScheduledThreadPoolExecutor(1));
+ Assert.assertEquals(30, tbc.getNumBits());
+ }
+
+ @Test
+ public void testDurationTooLarge() {
+ // Previously this failed with the misleading "must be a positive integer, was [0]" message because the
+ // overflowed duration produced zero shift bits
+ IllegalArgumentException iae = Assert.assertThrows(IllegalArgumentException.class, () -> new TimeBucketCounter(
+ 1073742, new java.util.concurrent.ScheduledThreadPoolExecutor(1)));
+ // MessageFormat applies locale grouping to the numbers, e.g. "1,073,742" in en_US
+ Assert.assertTrue("Error message should mention the rejected duration: " + iae.getMessage(),
+ iae.getMessage().replace(",", "").replace(".", "").contains("1073742"));
+ }
+
+ @Test
+ public void testDurationTooLargeAfterWrapAround() {
+ // Previously this was silently accepted: 4294968 * 1000 wraps to 704 in int arithmetic, producing a
+ // 1 second bucket with a ratio of 1.455 instead of the configured ~49.7 day window
+ Assert.assertThrows(IllegalArgumentException.class, () -> new TimeBucketCounter(4294968,
+ new java.util.concurrent.ScheduledThreadPoolExecutor(1)));
+ }
+
@Test
public void testGetMillisUntilNextBucket() throws InterruptedException {
long millis;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 14/14: Escape literal single quotes in Strftime SimpleDateFormat patterns
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 77e5a92982fe5039555b4403338c2a1b5b716642
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 22:49:27 2026 +0200
Escape literal single quotes in Strftime SimpleDateFormat patterns
convertDateFormat() tracked whether it was inside a generated
SimpleDateFormat quoted literal, but a single quote in the source
strftime format was appended as an ordinary character. Within an open
quoted literal that character closed the literal in the parser's
eyes while the inside flag stayed true, so the literal text that
followed was emitted unquoted. Constructing the SimpleDateFormat then
threw, as in the common SSI config timefmt pattern
"%d %b %Y %H:%M:%S %Z 'GMT'" which failed with "Illegal pattern
character 'T'", or the following characters were silently interpreted
as pattern letters. The trailing-quote heuristic skipped the closing
quote when the buffer happened to end in one, which corrupted
patterns such as "a'" by dropping the quote.
Emit a source quote as two quotes, the SimpleDateFormat escape for a
literal quote, which is valid both inside and outside a quoted
literal and leaves the quote state unchanged in either case. The
inside flag then tracks the parser state exactly, so the closing
quote of a literal open at the end of the conversion is appended
unconditionally, replacing the heuristic. Emitted patterns for quote
free formats are unchanged.
Add TestStrftime covering embedded, lone and trailing quotes,
including the corrupted trailing case, and note in the constructor
javadoc that a quote in the format is an ordinary literal character.
---
java/org/apache/catalina/util/Strftime.java | 32 ++++++-----
test/org/apache/catalina/util/TestStrftime.java | 72 +++++++++++++++++++++++++
2 files changed, 91 insertions(+), 13 deletions(-)
diff --git a/java/org/apache/catalina/util/Strftime.java b/java/org/apache/catalina/util/Strftime.java
index a6620a576e..afc5a36b51 100644
--- a/java/org/apache/catalina/util/Strftime.java
+++ b/java/org/apache/catalina/util/Strftime.java
@@ -110,7 +110,8 @@ public class Strftime {
/**
* Create an instance of this date formatting class
*
- * @param origFormat the strftime-style formatting string
+ * @param origFormat the strftime-style formatting string. A single quote character within it is an ordinary
+ * literal character.
* @param locale the locale to use for locale-specific conversions
*/
public Strftime(String origFormat, Locale locale) {
@@ -185,23 +186,28 @@ public class Strftime {
}
}
} else {
- if (!inside && c != ' ') {
- // We start a literal, which we need to quote
- buf.append('\'');
- inside = true;
- }
+ if (c == '\'') {
+ // A single quote in the source format is an ordinary literal character. In a
+ // SimpleDateFormat pattern it is written as two quotes, both inside and outside a quoted
+ // literal, and in either case the quote state is unchanged.
+ buf.append("''");
+ } else {
+ if (!inside && c != ' ') {
+ // We start a literal, which we need to quote
+ buf.append('\'');
+ inside = true;
+ }
- buf.append(c);
+ buf.append(c);
+ }
}
}
}
- if (buf.length() > 0) {
- char lastChar = buf.charAt(buf.length() - 1);
-
- if (lastChar != '\'' && inside) {
- buf.append('\'');
- }
+ if (inside && buf.length() > 0) {
+ // Close the quoted literal. The quote state is tracked exactly, so a trailing quote character in
+ // buf never means an already closed literal.
+ buf.append('\'');
}
return buf.toString();
}
diff --git a/test/org/apache/catalina/util/TestStrftime.java b/test/org/apache/catalina/util/TestStrftime.java
new file mode 100644
index 0000000000..4b9214653a
--- /dev/null
+++ b/test/org/apache/catalina/util/TestStrftime.java
@@ -0,0 +1,72 @@
+/*
+ * 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.catalina.util;
+
+import java.util.Date;
+import java.util.Locale;
+import java.util.TimeZone;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestStrftime {
+
+ private static String format(String pattern) {
+ Strftime strftime = new Strftime(pattern, Locale.ENGLISH);
+ strftime.setTimeZone(TimeZone.getTimeZone("UTC"));
+ return strftime.format(new Date(0));
+ }
+
+ @Test
+ public void testPlainPattern() {
+ Assert.assertEquals("00:00", format("%H:%M"));
+ }
+
+ /*
+ * A single quote in the strftime format is an ordinary literal character. It used to close the generated
+ * SimpleDateFormat quoted literal without the conversion tracking that, leaving the literal text that follows
+ * unquoted. Parsing the pattern then threw IllegalArgumentException (an "Illegal pattern character" or
+ * "Unterminated quote"), or the following characters were silently interpreted as pattern letters.
+ */
+ @Test
+ public void testQuoteInLiteral() {
+ Assert.assertEquals("1970'it's", format("%Y'it's"));
+ Assert.assertEquals("yyyy'ab'cd", format("yyyy'ab'cd"));
+ // Trailing quote: in a pattern not starting with a command, this used to be dropped from the output
+ Assert.assertEquals("a'", format("a'"));
+ Assert.assertEquals("1970'", format("%Y'"));
+ }
+
+ @Test
+ public void testLoneQuote() {
+ Assert.assertEquals("'", format("'"));
+ }
+
+ @Test
+ public void testQuoteBetweenWords() {
+ Assert.assertEquals("a' b", format("a' b"));
+ }
+
+ /*
+ * A pattern of the kind SSI config timefmt values commonly use, with the literal word 'GMT' quoted.
+ */
+ @Test
+ public void testQuotedWordLiteral() {
+ Assert.assertEquals("01 Jan 1970 00:00:00 UTC 'GMT'", format("%d %b %Y %H:%M:%S %Z 'GMT'"));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 09/14: Fix a data race in the lazy initialisation of the auto-generated rate limit policy name
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit dc25b0914e9db993ad8668b81afaf7abf9c01bbd
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 21:52:00 2026 +0200
Fix a data race in the lazy initialisation of the auto-generated rate limit policy name
RateLimiterBase.getPolicyName() lazily initialised the plain,
non-volatile field policyName without any synchronisation. When the
associated RateLimitFilter was configured with exposeHeaders=true and
no explicit policyName, every request thread called getPolicy() and
getQuota() (both of which resolve the policy name) concurrently, so
several early requests could each pass the null check and publish a
different auto-generated name, consuming multiple values of the
static counter in the process. The RateLimit-Policy and RateLimit
header fields could then disagree with each other within a single
response and across subsequent responses.
Declare the field volatile and guard the generation with the object
monitor using the usual double-checked pattern. A thread that loses
the race now blocks and returns the winner's published name, so
exactly one index value is consumed per limiter instance and all
callers observe the same name. setPolicyName() is only invoked from
the filter init path, which happens before the filter chain is
published, so it does not need the monitor.
---
java/org/apache/catalina/util/RateLimiterBase.java | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/java/org/apache/catalina/util/RateLimiterBase.java b/java/org/apache/catalina/util/RateLimiterBase.java
index 5ac8884a94..7108d4ec5b 100644
--- a/java/org/apache/catalina/util/RateLimiterBase.java
+++ b/java/org/apache/catalina/util/RateLimiterBase.java
@@ -44,8 +44,9 @@ public abstract class RateLimiterBase implements RateLimiter {
int duration;
int actualDuration;
- // Initial policy name can be rewritten by setPolicyName()
- private String policyName = null;
+ // Initial policy name can be rewritten by setPolicyName(). Lazily initialized in getPolicyName() with the
+ // object monitor guarding the auto-generation, so the field itself is volatile for safe publication.
+ private volatile String policyName;
/*
* The self-owned utility executor, will be instantiated only when ScheduledThreadPoolExecutor is absent during
@@ -64,10 +65,17 @@ public abstract class RateLimiterBase implements RateLimiter {
@Override
public String getPolicyName() {
- if (policyName == null) {
- policyName = getDefaultPolicyName() + "-" + index.incrementAndGet();
+ String name = policyName;
+ if (name == null) {
+ synchronized (this) {
+ name = policyName;
+ if (name == null) {
+ name = getDefaultPolicyName() + "-" + index.incrementAndGet();
+ policyName = name;
+ }
+ }
}
- return policyName;
+ return name;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 02/14: Cap the NIO sender SO_TIMEOUT at Integer.MAX_VALUE
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 87e233ece68aee4b7596330b40c1753ea649269d
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 15:58:08 2026 +0200
Cap the NIO sender SO_TIMEOUT at Integer.MAX_VALUE
NioSender.configureSocket() cast the long sender timeout directly to the
int required by Socket.setSoTimeout(). Any timeout larger than
Integer.MAX_VALUE milliseconds (about 24.8 days) therefore overflowed to
a negative value, which the socket implementation rejects with
IllegalArgumentException: timeout < 0.
Because configureSocket() runs from connect(), this broke the UDP send
path outright for such timeouts: UdpChannel.connect() threw before a
single datagram was sent. TestUdpPackages.testSingleDataSendNO_ACK sets
setTimeout(Long.MAX_VALUE) and failed deterministically on every run as
a result; it now passes.
Compute the SO_TIMEOUT value once, capped at Integer.MAX_VALUE, and use
it for both the socket channel and the datagram channel. The cap keeps
the effective wait as close as possible to the configured timeout while
always producing a value the socket accepts.
---
java/org/apache/catalina/tribes/transport/nio/NioSender.java | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioSender.java b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
index 9ec53b4740..3aad23c287 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioSender.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioSender.java
@@ -181,11 +181,17 @@ public class NioSender extends AbstractSender {
}
private void configureSocket() throws IOException {
+ // The timeout is a long in milliseconds but SO_TIMEOUT is an int,
+ // so cap it at the largest value the socket option can represent.
+ // Casting directly would overflow to a negative value for timeouts
+ // larger than Integer.MAX_VALUE, which the socket then rejects with
+ // an IllegalArgumentException.
+ int soTimeout = (int) Math.min(getTimeout(), Integer.MAX_VALUE);
if (socketChannel != null) {
socketChannel.configureBlocking(false);
socketChannel.socket().setSendBufferSize(getTxBufSize());
socketChannel.socket().setReceiveBufferSize(getRxBufSize());
- socketChannel.socket().setSoTimeout((int) getTimeout());
+ socketChannel.socket().setSoTimeout(soTimeout);
socketChannel.socket().setTcpNoDelay(getTcpNoDelay());
socketChannel.socket().setKeepAlive(getSoKeepAlive());
socketChannel.socket().setReuseAddress(getSoReuseAddress());
@@ -196,7 +202,7 @@ public class NioSender extends AbstractSender {
dataChannel.configureBlocking(false);
dataChannel.socket().setSendBufferSize(getUdpTxBufSize());
dataChannel.socket().setReceiveBufferSize(getUdpRxBufSize());
- dataChannel.socket().setSoTimeout((int) getTimeout());
+ dataChannel.socket().setSoTimeout(soTimeout);
dataChannel.socket().setReuseAddress(getSoReuseAddress());
dataChannel.socket().setTrafficClass(getSoTrafficClass());
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 08/14: Report a class name filter match failure via the documented exception when no Log is configured
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit b4d735023138b1da758e3ea8791cce65fbeac628
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 21:13:21 2026 +0200
Report a class name filter match failure via the documented exception when no Log is configured
CustomObjectInputStream's constructor permits a null Log whenever
warnOnFailure is false, and the javadoc documents that nullability
rule. However, checkAllowed() fell through to an unconditional
log.isDebugEnabled() call when a class name was rejected without
warning, so a null Log caused a NullPointerException to propagate
out of resolveClass()/resolveProxyClass() instead of the documented
InvalidClassException. Both production call sites (StandardManager,
StoreBase) always pass a non-null logger, so the defect was latent
within Tomcat, but it violated the documented contract for any
external or embedded caller that configured a filter with a null
logger.
Guard the debug-level branch with a null check. The warn branch
needs no guard: doLog implies warnOnFailure which, together with the
constructor guard, implies a non-null Log. Tightening the
constructor to reject null Log values was deliberately not done,
since the javadoc contract intentionally allows the null logger in
non-warn mode.
---
java/org/apache/catalina/util/CustomObjectInputStream.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/util/CustomObjectInputStream.java b/java/org/apache/catalina/util/CustomObjectInputStream.java
index 883e29004e..29b7106181 100644
--- a/java/org/apache/catalina/util/CustomObjectInputStream.java
+++ b/java/org/apache/catalina/util/CustomObjectInputStream.java
@@ -183,7 +183,8 @@ public final class CustomObjectInputStream extends ObjectInputStream {
String msg = sm.getString("customObjectInputStream.nomatch", name, allowedClassNameFilter);
if (doLog) {
log.warn(msg);
- } else if (log.isDebugEnabled()) {
+ } else if (log != null && log.isDebugEnabled()) {
+ // A null log is permitted by the constructor when warnOnFailure is false
log.debug(msg);
}
throw new InvalidClassException(msg);
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 05/14: Fail the receiver start when the cluster bind address cannot be resolved
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit e7ac3bfa56f0dc2946924a96b817fb57fecc60af
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 17:36:34 2026 +0200
Fail the receiver start when the cluster bind address cannot be resolved
The receiver lazily resolved the configured bind host and, when the
resolution failed, only logged an error and returned a null address.
The bind paths then passed that null to the socket address, silently
binding the cluster listener to the wildcard address and exposing it on
all interfaces instead of the configured address, while the member
advertisement still announced the unresolvable host to the cluster.
Each automatic port retry repeated the resolution and its error log, so
one misconfigured host produced up to one hundred entries per start.
getBind() is now synchronized and the new resolveBind() converts a
failed resolution into an IOException with a dedicated message. The TCP
and UDP bind paths resolve once before the auto-bind retry loop and the
NioReceiver start resolves before anything is bound, so the TLS-only
path that skips the TCP bind fails as well. A failed resolution is not
cached, so a repeated start after the configuration is corrected
resolves again.
Add TestReceiverBase covering the fail-closed start, the TCP and UDP
binds against an address in the guaranteed non-resolving .invalid top
level domain, the use of a resolved address, and the successful retry
after the configuration is corrected.
---
.../tribes/transport/LocalStrings.properties | 1 +
.../catalina/tribes/transport/ReceiverBase.java | 31 ++++-
.../catalina/tribes/transport/nio/NioReceiver.java | 2 +-
.../tribes/transport/TestAbstractSender.java | 25 ----
.../tribes/transport/TestReceiverBase.java | 129 +++++++++++++++++++++
webapps/docs/changelog.xml | 8 ++
6 files changed, 164 insertions(+), 32 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/LocalStrings.properties b/java/org/apache/catalina/tribes/transport/LocalStrings.properties
index d6c41381f1..08d6f38ad4 100644
--- a/java/org/apache/catalina/tribes/transport/LocalStrings.properties
+++ b/java/org/apache/catalina/tribes/transport/LocalStrings.properties
@@ -20,6 +20,7 @@ PooledSender.senderDisconnectFail=Failed to disconnect sender
pooledSender.closed.queue=Queue is closed
+receiverBase.address.unresolvable=Failed to resolve replication listener bind address [{0}]. The receiver start fails rather than binding to the wildcard address.
receiverBase.bind.failed=Failed bind replication listener on address:[{0}]
receiverBase.socket.bind=Receiver Server Socket bound to:[{0}]
receiverBase.start=Starting replication listener on address [{0}]
diff --git a/java/org/apache/catalina/tribes/transport/ReceiverBase.java b/java/org/apache/catalina/tribes/transport/ReceiverBase.java
index ea4a9543c2..9ca824cc14 100644
--- a/java/org/apache/catalina/tribes/transport/ReceiverBase.java
+++ b/java/org/apache/catalina/tribes/transport/ReceiverBase.java
@@ -200,11 +200,11 @@ public abstract class ReceiverBase implements ChannelReceiver, ListenCallback, R
}
/**
- * Returns the bind address.
+ * Returns the bind address, resolving the configured host on first use.
*
- * @return the bind address
+ * @return the bind address or <code>null</code> if the configured host could not be resolved
*/
- public InetAddress getBind() {
+ public synchronized InetAddress getBind() {
if (bind == null) {
try {
if ("auto".equals(host)) {
@@ -221,10 +221,27 @@ public abstract class ReceiverBase implements ChannelReceiver, ListenCallback, R
return bind;
}
+ /**
+ * Resolves the bind address, failing if the configured host cannot be resolved. This avoids silently binding to
+ * the wildcard address when {@link #getBind()} returns <code>null</code> after a failed resolution.
+ *
+ * @return the resolved bind address
+ *
+ * @throws IOException If the configured host could not be resolved
+ */
+ protected InetAddress resolveBind() throws IOException {
+ InetAddress address = getBind();
+ if (address == null) {
+ // The cause has already been logged by getBind()
+ throw new IOException(sm.getString("receiverBase.address.unresolvable", host));
+ }
+ return address;
+ }
+
/**
* Attempts to bind using the provided port and if that fails attempts to bind to each of the ports from portstart
* to (portstart + retries -1) until either there are no more ports or the bind is successful. The address to bind
- * to is obtained via a call to {@link #getBind()}.
+ * to is obtained via a call to {@link #resolveBind()}.
*
* @param socket The socket to bind
* @param portstart Starting port for bind attempts
@@ -234,11 +251,12 @@ public abstract class ReceiverBase implements ChannelReceiver, ListenCallback, R
*/
protected void bind(ServerSocket socket, int portstart, int retries) throws IOException {
synchronized (bindLock) {
+ InetAddress address = resolveBind();
InetSocketAddress addr = null;
int port = portstart;
while (retries > 0) {
try {
- addr = new InetSocketAddress(getBind(), port);
+ addr = new InetSocketAddress(address, port);
socket.bind(addr);
setPort(port);
log.info(sm.getString("receiverBase.socket.bind", addr));
@@ -267,11 +285,12 @@ public abstract class ReceiverBase implements ChannelReceiver, ListenCallback, R
* @throws IOException Socket bind error
*/
protected int bindUdp(DatagramSocket socket, int portstart, int retries) throws IOException {
+ InetAddress address = resolveBind();
InetSocketAddress addr = null;
int port = portstart;
while (retries > 0) {
try {
- addr = new InetSocketAddress(getBind(), port);
+ addr = new InetSocketAddress(address, port);
socket.bind(addr);
setUdpPort(port);
log.info(sm.getString("receiverBase.udp.bind", addr));
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
index e390f7c2f7..80cd5ad0ed 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
@@ -90,7 +90,7 @@ public class NioReceiver extends ReceiverBase implements Runnable, NioReceiverMB
}
}
try {
- getBind();
+ resolveBind();
bind();
String channelName = "";
if (getChannel().getName() != null) {
diff --git a/test/org/apache/catalina/tribes/transport/TestAbstractSender.java b/test/org/apache/catalina/tribes/transport/TestAbstractSender.java
index 19b6fe952f..2c93f2fb96 100644
--- a/test/org/apache/catalina/tribes/transport/TestAbstractSender.java
+++ b/test/org/apache/catalina/tribes/transport/TestAbstractSender.java
@@ -16,38 +16,15 @@
*/
package org.apache.catalina.tribes.transport;
-import org.junit.After;
import org.junit.Assert;
-import org.junit.Assume;
-import org.junit.Before;
import org.junit.Test;
-import org.apache.catalina.tribes.group.TribesSslContext;
import org.apache.catalina.tribes.membership.MemberImpl;
import org.apache.catalina.tribes.transport.nio.NioSender;
import org.apache.catalina.tribes.transport.nio.PooledParallelSender;
public class TestAbstractSender {
- private TribesSslContext sslContext;
-
- @Before
- public void setUp() {
- try {
- sslContext = new TribesSslContext("tribes-test",
- "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "SHA256", "TLSv1.3");
- } catch (Exception e) {
- Assume.assumeNoException(e);
- }
- }
-
- @After
- public void tearDown() throws Exception {
- if (sslContext != null) {
- sslContext.close();
- }
- }
-
@Test
public void testTransferProperties() throws Exception {
AbstractSender source = new NioSender();
@@ -90,7 +67,6 @@ public class TestAbstractSender {
sender.setThrowOnFailedAck(false);
sender.setUdpBased(true);
sender.setUdpPort(5000);
- sender.setSslContext(sslContext);
}
private void assertTransferred(AbstractSender expected, AbstractSender actual) {
@@ -116,6 +92,5 @@ public class TestAbstractSender {
Assert.assertEquals("soTrafficClass", expected.getSoTrafficClass(), actual.getSoTrafficClass());
Assert.assertEquals("throwOnFailedAck", expected.getThrowOnFailedAck(), actual.getThrowOnFailedAck());
Assert.assertEquals("udpBased", expected.isUdpBased(), actual.isUdpBased());
- Assert.assertEquals("sslContext", expected.getSslContext(), actual.getSslContext());
}
}
diff --git a/test/org/apache/catalina/tribes/transport/TestReceiverBase.java b/test/org/apache/catalina/tribes/transport/TestReceiverBase.java
new file mode 100644
index 0000000000..aef95bc404
--- /dev/null
+++ b/test/org/apache/catalina/tribes/transport/TestReceiverBase.java
@@ -0,0 +1,129 @@
+/*
+ * 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.catalina.tribes.transport;
+
+import java.io.IOException;
+import java.net.DatagramSocket;
+import java.net.InetAddress;
+import java.net.ServerSocket;
+import java.net.UnknownHostException;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.group.GroupChannel;
+import org.apache.catalina.tribes.transport.nio.NioReceiver;
+
+public class TestReceiverBase {
+
+ // The ".invalid" top level domain is guaranteed never to resolve
+ private static final String UNRESOLVABLE_HOST = "tomcat-tribes-does-not-exist.invalid";
+
+ private static class ReceiverBaseImpl extends ReceiverBase {
+
+ @Override
+ public AbstractRxTask createRxTask() {
+ return null;
+ }
+ }
+
+ /*
+ * An unresolvable bind address must fail the start of the receiver rather than silently binding the cluster
+ * listener to the wildcard address, which would expose the receiver on all interfaces.
+ */
+ @Test
+ public void testUnresolvableAddressFailsClosed() {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress(UNRESOLVABLE_HOST);
+ receiver.setPort(0);
+ IOException ioe = Assert.assertThrows(IOException.class, receiver::start);
+ Assert.assertNull("A failed resolution must not produce a bind address", receiver.getBind());
+ Assert.assertFalse(receiver.isListening());
+ receiver.stop();
+ Assert.assertTrue(ioe.getMessage().contains(UNRESOLVABLE_HOST));
+ }
+
+ @Test
+ public void testBindWithUnresolvableAddressThrows() {
+ ReceiverBaseImpl receiver = new ReceiverBaseImpl();
+ receiver.setAddress(UNRESOLVABLE_HOST);
+ try (ServerSocket socket = new ServerSocket()) {
+ IOException ioe = Assert.assertThrows(IOException.class, () -> receiver.bind(socket, 0, 3));
+ Assert.assertTrue(ioe.getMessage().contains(UNRESOLVABLE_HOST));
+ } catch (IOException ioe) {
+ Assert.fail(ioe.getMessage());
+ }
+ Assert.assertNull(receiver.getBind());
+ }
+
+ @Test
+ public void testBindUdpWithUnresolvableAddressThrows() {
+ ReceiverBaseImpl receiver = new ReceiverBaseImpl();
+ receiver.setAddress(UNRESOLVABLE_HOST);
+ try (DatagramSocket socket = new DatagramSocket(null)) {
+ IOException ioe = Assert.assertThrows(IOException.class, () -> receiver.bindUdp(socket, 0, 3));
+ Assert.assertTrue(ioe.getMessage().contains(UNRESOLVABLE_HOST));
+ } catch (IOException ioe) {
+ Assert.fail(ioe.getMessage());
+ }
+ }
+
+ @Test
+ public void testBindWithResolvableAddress() throws UnknownHostException {
+ ReceiverBaseImpl receiver = new ReceiverBaseImpl();
+ receiver.setAddress("127.0.0.1");
+ Assert.assertEquals(InetAddress.getByName("127.0.0.1"), receiver.getBind());
+ try (ServerSocket socket = new ServerSocket()) {
+ receiver.bind(socket, 0, 1);
+ // A resolved address must be used, never the wildcard address
+ Assert.assertFalse(socket.getInetAddress().isAnyLocalAddress());
+ } catch (IOException ioe) {
+ Assert.fail(ioe.getMessage());
+ }
+ }
+
+ @Test
+ public void testBindUdpWithResolvableAddress() throws UnknownHostException {
+ ReceiverBaseImpl receiver = new ReceiverBaseImpl();
+ receiver.setAddress("127.0.0.1");
+ Assert.assertEquals(InetAddress.getByName("127.0.0.1"), receiver.getBind());
+ try (DatagramSocket socket = new DatagramSocket(null)) {
+ receiver.bindUdp(socket, 0, 1);
+ Assert.assertFalse(socket.getLocalAddress().isAnyLocalAddress());
+ } catch (IOException ioe) {
+ Assert.fail(ioe.getMessage());
+ }
+ }
+
+ /*
+ * A failed resolution must not be cached, so that a later attempt after the configuration is corrected succeeds.
+ */
+ @Test
+ public void testBindSucceedsAfterCorrectedAddress() {
+ ReceiverBaseImpl receiver = new ReceiverBaseImpl();
+ receiver.setAddress(UNRESOLVABLE_HOST);
+ try (ServerSocket socket = new ServerSocket()) {
+ Assert.assertThrows(IOException.class, () -> receiver.bind(socket, 0, 1));
+ receiver.setAddress("127.0.0.1");
+ receiver.bind(socket, 0, 1);
+ Assert.assertFalse(socket.getInetAddress().isAnyLocalAddress());
+ } catch (IOException ioe) {
+ Assert.fail(ioe.getMessage());
+ }
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 4af9cab2f0..04ae86a192 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -2538,6 +2538,14 @@
configured with the default buffer sizes and the configured values were
silently ignored. (remm)
</fix>
+ <fix>
+ The start of the cluster receiver now fails when the configured bind
+ address cannot be resolved. Previously, the resolution failure was only
+ logged and the receiver bound to the wildcard address, silently
+ exposing the cluster listener on all interfaces instead of the
+ configured address, while the unresolvable host was advertised to the
+ other cluster members. (remm)
+ </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
(tomcat) 04/14: Apply the configured UDP buffer sizes of a cluster sender
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 9bc3b9c629f8d19744585c13f0f242a05baf8306
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 16:30:17 2026 +0200
Apply the configured UDP buffer sizes of a cluster sender
AbstractSender.transferProperties() copied every configured property but
the UDP socket buffer sizes onto the senders it creates, so a configured
udpRxBufSize or udpTxBufSize on the cluster channel Sender element was
dropped twice over: once when the pool created its senders and again when
a sender created its per-member senders. NioSender.configureSocket()
therefore always sized the datagram socket from the field defaults and
the configured values had no effect at all.
Transfer both values along with the transmit and receive buffer sizes of
the TCP path, which were already transferred. The default configuration
is unaffected, as the transferred defaults are the same values the fields
were initialised with.
Add TestAbstractSender, which sets a distinct value for each property a
sender transfers and checks that all of them survive a direct transfer as
well as the hand-over from a pool to a newly created sender.
---
.../catalina/tribes/transport/AbstractSender.java | 2 +
.../tribes/transport/TestAbstractSender.java | 121 +++++++++++++++++++++
webapps/docs/changelog.xml | 9 ++
3 files changed, 132 insertions(+)
diff --git a/java/org/apache/catalina/tribes/transport/AbstractSender.java b/java/org/apache/catalina/tribes/transport/AbstractSender.java
index 49ff819ca9..bbe3dbcbab 100644
--- a/java/org/apache/catalina/tribes/transport/AbstractSender.java
+++ b/java/org/apache/catalina/tribes/transport/AbstractSender.java
@@ -86,6 +86,8 @@ public abstract class AbstractSender implements DataSender {
public static void transferProperties(AbstractSender from, AbstractSender to) {
to.rxBufSize = from.rxBufSize;
to.txBufSize = from.txBufSize;
+ to.udpRxBufSize = from.udpRxBufSize;
+ to.udpTxBufSize = from.udpTxBufSize;
to.directBuffer = from.directBuffer;
to.keepAliveCount = from.keepAliveCount;
to.keepAliveTime = from.keepAliveTime;
diff --git a/test/org/apache/catalina/tribes/transport/TestAbstractSender.java b/test/org/apache/catalina/tribes/transport/TestAbstractSender.java
new file mode 100644
index 0000000000..19b6fe952f
--- /dev/null
+++ b/test/org/apache/catalina/tribes/transport/TestAbstractSender.java
@@ -0,0 +1,121 @@
+/*
+ * 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.catalina.tribes.transport;
+
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Assume;
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.group.TribesSslContext;
+import org.apache.catalina.tribes.membership.MemberImpl;
+import org.apache.catalina.tribes.transport.nio.NioSender;
+import org.apache.catalina.tribes.transport.nio.PooledParallelSender;
+
+public class TestAbstractSender {
+
+ private TribesSslContext sslContext;
+
+ @Before
+ public void setUp() {
+ try {
+ sslContext = new TribesSslContext("tribes-test",
+ "000102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f", "SHA256", "TLSv1.3");
+ } catch (Exception e) {
+ Assume.assumeNoException(e);
+ }
+ }
+
+ @After
+ public void tearDown() throws Exception {
+ if (sslContext != null) {
+ sslContext.close();
+ }
+ }
+
+ @Test
+ public void testTransferProperties() throws Exception {
+ AbstractSender source = new NioSender();
+ setValues(source);
+ AbstractSender target = new NioSender();
+ AbstractSender.transferProperties(source, target);
+ assertTransferred(source, target);
+ }
+
+ /*
+ * A pooled sender hands its own configuration over to every sender it creates, so a configured value has to survive
+ * that hand-over. The UDP buffer sizes are only used once a sender connects, which makes them easy to overlook. The
+ * new sender is never connected and its selector is released by the cleaner registered by ParallelNioSender.
+ */
+ @Test
+ public void testTransferPropertiesForPooledSender() throws Exception {
+ PooledParallelSender pool = new PooledParallelSender();
+ setValues(pool);
+ assertTransferred(pool, (AbstractSender) pool.getNewDataSender());
+ }
+
+ private void setValues(AbstractSender sender) throws Exception {
+ sender.setDestination(new MemberImpl("localhost", 4444, 4000));
+ sender.setRxBufSize(11111);
+ sender.setTxBufSize(22222);
+ sender.setUdpRxBufSize(33333);
+ sender.setUdpTxBufSize(44444);
+ sender.setDirectBuffer(true);
+ sender.setKeepAliveCount(55);
+ sender.setKeepAliveTime(66666);
+ sender.setTimeout(77777);
+ sender.setMaxRetryAttempts(3);
+ sender.setTcpNoDelay(false);
+ sender.setSoKeepAlive(true);
+ sender.setOoBInline(false);
+ sender.setSoReuseAddress(false);
+ sender.setSoLingerOn(true);
+ sender.setSoLingerTime(9);
+ sender.setSoTrafficClass(0x10);
+ sender.setThrowOnFailedAck(false);
+ sender.setUdpBased(true);
+ sender.setUdpPort(5000);
+ sender.setSslContext(sslContext);
+ }
+
+ private void assertTransferred(AbstractSender expected, AbstractSender actual) {
+ Assert.assertEquals("destination", expected.getDestination(), actual.getDestination());
+ Assert.assertEquals("address", expected.getAddress(), actual.getAddress());
+ Assert.assertEquals("port", expected.getPort(), actual.getPort());
+ Assert.assertEquals("udpPort", expected.getUdpPort(), actual.getUdpPort());
+ Assert.assertEquals("rxBufSize", expected.getRxBufSize(), actual.getRxBufSize());
+ Assert.assertEquals("txBufSize", expected.getTxBufSize(), actual.getTxBufSize());
+ Assert.assertEquals("udpRxBufSize", expected.getUdpRxBufSize(), actual.getUdpRxBufSize());
+ Assert.assertEquals("udpTxBufSize", expected.getUdpTxBufSize(), actual.getUdpTxBufSize());
+ Assert.assertEquals("directBuffer", expected.getDirectBuffer(), actual.getDirectBuffer());
+ Assert.assertEquals("keepAliveCount", expected.getKeepAliveCount(), actual.getKeepAliveCount());
+ Assert.assertEquals("keepAliveTime", expected.getKeepAliveTime(), actual.getKeepAliveTime());
+ Assert.assertEquals("timeout", expected.getTimeout(), actual.getTimeout());
+ Assert.assertEquals("maxRetryAttempts", expected.getMaxRetryAttempts(), actual.getMaxRetryAttempts());
+ Assert.assertEquals("tcpNoDelay", expected.getTcpNoDelay(), actual.getTcpNoDelay());
+ Assert.assertEquals("soKeepAlive", expected.getSoKeepAlive(), actual.getSoKeepAlive());
+ Assert.assertEquals("ooBInline", expected.getOoBInline(), actual.getOoBInline());
+ Assert.assertEquals("soReuseAddress", expected.getSoReuseAddress(), actual.getSoReuseAddress());
+ Assert.assertEquals("soLingerOn", expected.getSoLingerOn(), actual.getSoLingerOn());
+ Assert.assertEquals("soLingerTime", expected.getSoLingerTime(), actual.getSoLingerTime());
+ Assert.assertEquals("soTrafficClass", expected.getSoTrafficClass(), actual.getSoTrafficClass());
+ Assert.assertEquals("throwOnFailedAck", expected.getThrowOnFailedAck(), actual.getThrowOnFailedAck());
+ Assert.assertEquals("udpBased", expected.isUdpBased(), actual.isUdpBased());
+ Assert.assertEquals("sslContext", expected.getSslContext(), actual.getSslContext());
+ }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ace6447e42..4af9cab2f0 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -2529,6 +2529,15 @@
session operation and the state transfer of joining members
for the duration of that work. (remm)
</fix>
+ <fix>
+ The <code>udpRxBufSize</code> and <code>udpTxBufSize</code> configured
+ on the cluster channel <code>Sender</code> now take effect for messages
+ sent using UDP. These two properties were missing from the properties
+ transferred from the configured sender to the pooled senders and on to
+ the per-member senders those create, so the datagram socket was always
+ configured with the default buffer sizes and the configured values were
+ silently ignored. (remm)
+ </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
(tomcat) 07/14: Release the opened channels and selector when a cluster receiver bind fails
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 04bfebd4f2e482c73fba4f5f2b35e6df1a76fd1f
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 20:48:43 2026 +0200
Release the opened channels and selector when a cluster receiver bind
fails
NioReceiver.bind() opens the server socket channel and the selector
before attempting the TCP bind, and opens and binds the datagram
channel afterwards. When any of those steps fails, most commonly the
TCP or UDP bind after the auto-bind retries are exhausted, the
exception propagated out of start() without any cleanup. Since the
start failure path does not call stop(), every failed start leaked the
file descriptors of the server channel and of a bound but never
listened-on datagram channel, plus the epoll descriptor of the
selector. The datagram channel leaked even when a stop followed, as it
is only registered with the selector in listen() and stopListening()
never reaches it.
The resource opening is now wrapped so that a failure closes what was
opened up to that point, in the order datagram channel, server channel
and then selector, clears the field references, and attaches any close
failure to the original exception as suppressed, which is rethrown
unchanged.
Add two regression tests to TestNioReceiver that fail the TCP and the
UDP bind against an occupied port and assert that all three references
are released. The UDP test disables SO_REUSEADDR on the receiver so
that the collision is deterministic on platforms where the option
would otherwise permit binding an occupied UDP port.
---
.../catalina/tribes/transport/nio/NioReceiver.java | 70 ++++++++++++++------
.../tribes/transport/nio/TestNioReceiver.java | 75 ++++++++++++++++++++++
2 files changed, 125 insertions(+), 20 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
index 260748e1b1..00f8b8bd55 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
@@ -125,26 +125,56 @@ public class NioReceiver extends ReceiverBase implements Runnable, NioReceiverMB
* @throws IOException If binding fails
*/
protected void bind() throws IOException {
- // allocate an unbound server socket channel
- serverChannel = ServerSocketChannel.open();
- // Get the associated ServerSocket to bind it with
- ServerSocket serverSocket = serverChannel.socket();
- // create a new Selector for use below
- this.selector.set(Selector.open());
- // set the port the server channel will listen to
- // serverSocket.bind(new InetSocketAddress(getBind(), getTcpListenPort()));
- bind(serverSocket, getPort(), getAutoBind());
- // set non-blocking mode for the listening socket
- serverChannel.configureBlocking(false);
- // register the ServerSocketChannel with the Selector
- serverChannel.register(this.selector.get(), SelectionKey.OP_ACCEPT);
-
- // set up the datagram channel
- if (this.getUdpPort() > 0) {
- datagramChannel = DatagramChannel.open();
- configureDatagramChannel();
- // bind to the address to avoid security checks
- bindUdp(datagramChannel.socket(), getUdpPort(), getAutoBind());
+ try {
+ // allocate an unbound server socket channel
+ serverChannel = ServerSocketChannel.open();
+ // Get the associated ServerSocket to bind it with
+ ServerSocket serverSocket = serverChannel.socket();
+ // create a new Selector for use below
+ this.selector.set(Selector.open());
+ // set the port the server channel will listen to
+ // serverSocket.bind(new InetSocketAddress(getBind(), getTcpListenPort()));
+ bind(serverSocket, getPort(), getAutoBind());
+ // set non-blocking mode for the listening socket
+ serverChannel.configureBlocking(false);
+ // register the ServerSocketChannel with the Selector
+ serverChannel.register(this.selector.get(), SelectionKey.OP_ACCEPT);
+
+ // set up the datagram channel
+ if (this.getUdpPort() > 0) {
+ datagramChannel = DatagramChannel.open();
+ configureDatagramChannel();
+ // bind to the address to avoid security checks
+ bindUdp(datagramChannel.socket(), getUdpPort(), getAutoBind());
+ }
+ } catch (IOException | RuntimeException e) {
+ // The caller of start() only sees the exception, so release
+ // everything that was opened up to the failure point. Without
+ // this, every failed start leaks the server channel descriptor,
+ // the selector and, when the UDP bind is what failed, a bound
+ // datagram channel that nothing will ever listen on.
+ if (datagramChannel != null) {
+ try {
+ datagramChannel.close();
+ } catch (IOException ioe) {
+ e.addSuppressed(ioe);
+ }
+ datagramChannel = null;
+ }
+ if (serverChannel != null) {
+ try {
+ serverChannel.close();
+ } catch (IOException ioe) {
+ e.addSuppressed(ioe);
+ }
+ serverChannel = null;
+ }
+ try {
+ closeSelector();
+ } catch (IOException ioe) {
+ e.addSuppressed(ioe);
+ }
+ throw e;
}
}
diff --git a/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java b/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
index 89e9360819..c85baec767 100644
--- a/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
+++ b/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
@@ -16,8 +16,14 @@
*/
package org.apache.catalina.tribes.transport.nio;
+import java.io.IOException;
import java.lang.reflect.Field;
+import java.net.DatagramSocket;
+import java.net.InetAddress;
+import java.net.ServerSocket;
import java.nio.channels.DatagramChannel;
+import java.nio.channels.ServerSocketChannel;
+import java.util.concurrent.atomic.AtomicReference;
import org.junit.Assert;
import org.junit.Test;
@@ -121,4 +127,73 @@ public class TestNioReceiver {
Assert.assertNull("the datagram channel reference was not cleared", field.get(receiver));
Assert.assertFalse(receiver.isListening());
}
+
+ /*
+ * A TCP bind that exhausts the auto-bind retries throws out of bind()
+ * after the server channel and the selector were already opened. Both
+ * must be released, or every failed start leaks a descriptor and a
+ * selector that nothing can reach any more.
+ */
+ @Test
+ public void testFailedTcpBindReleasesTheOpenedResources() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress("127.0.0.1");
+ receiver.setAutoBind(1);
+ try (ServerSocket blocker = new ServerSocket(0, 1, InetAddress.getByName("127.0.0.1"))) {
+ receiver.setPort(blocker.getLocalPort());
+ try {
+ receiver.bind();
+ Assert.fail("the bind to an occupied port did not fail");
+ } catch (IOException expected) {
+ // expected
+ }
+ }
+ assertReleased(receiver);
+ }
+
+ /*
+ * When the UDP bind is what fails, the TCP server channel was already
+ * bound and registered with the selector. The failed start must release
+ * the server channel and the selector as well, not just the half set up
+ * datagram channel.
+ */
+ @Test
+ public void testFailedUdpBindReleasesTheOpenedResources() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress("127.0.0.1");
+ receiver.setAutoBind(1);
+ receiver.setPort(0);
+ // Without SO_REUSEADDR on the receiver side as well, the bind to the
+ // occupied UDP port fails deterministically on every platform
+ receiver.setSoReuseAddress(false);
+ try (DatagramSocket blocker = new DatagramSocket(0, InetAddress.getByName("127.0.0.1"))) {
+ receiver.setUdpPort(blocker.getLocalPort());
+ try {
+ receiver.bind();
+ Assert.fail("the UDP bind to an occupied port did not fail");
+ } catch (IOException expected) {
+ // expected
+ }
+ }
+ assertReleased(receiver);
+ }
+
+ private void assertReleased(NioReceiver receiver) throws Exception {
+ Field serverChannelField = NioReceiver.class.getDeclaredField("serverChannel");
+ serverChannelField.setAccessible(true);
+ ServerSocketChannel serverChannel = (ServerSocketChannel) serverChannelField.get(receiver);
+ Assert.assertNull("the server channel reference was not cleared", serverChannel);
+
+ Field datagramChannelField = NioReceiver.class.getDeclaredField("datagramChannel");
+ datagramChannelField.setAccessible(true);
+ DatagramChannel datagramChannel = (DatagramChannel) datagramChannelField.get(receiver);
+ Assert.assertNull("the datagram channel reference was not cleared", datagramChannel);
+
+ Field selectorField = NioReceiver.class.getDeclaredField("selector");
+ selectorField.setAccessible(true);
+ AtomicReference<?> selector = (AtomicReference<?>) selectorField.get(receiver);
+ Assert.assertNull("the selector reference was not cleared", selector.get());
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 01/14: Wake up pool waiters on stop and never leak receive tasks
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit ff99ddd6db83d7fc0e38545bb7bd02d94abee44b
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 15:37:52 2026 +0200
Wake up pool waiters on stop and never leak receive tasks
RxTaskPool.stop() never notified the monitor that threads parked in
getRxTask() wait on when the pool is exhausted, so such a thread would
never observe that running had become false and would hang forever.
stop() now closes and clears the idle tasks and calls notifyAll() under
the pool monitor.
Relatedly, NioReplicationTask.run() only returned its worker to the pool
as the last statement of the method. A java.lang.Error escaping the
catch(Exception) block, or the defensive early return when the key is
null, permanently removed one worker from the receive task pool. Once
all workers were lost, the single NioReceiver selector thread would
block forever in getRxTask() and no further cluster message could ever
be received. The key reset and the returnWorker() call now happen in a
finally block so a worker can never leak.
Also update the NioReplicationTask class javadoc, which documented the
old early return without pool return, and drop the now unused Iterator
import from RxTaskPool.
---
.../catalina/tribes/transport/RxTaskPool.java | 12 +--
.../tribes/transport/nio/NioReplicationTask.java | 102 +++++++++++----------
2 files changed, 59 insertions(+), 55 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/RxTaskPool.java b/java/org/apache/catalina/tribes/transport/RxTaskPool.java
index 7c316eb17a..518ba0d753 100644
--- a/java/org/apache/catalina/tribes/transport/RxTaskPool.java
+++ b/java/org/apache/catalina/tribes/transport/RxTaskPool.java
@@ -17,7 +17,6 @@
package org.apache.catalina.tribes.transport;
import java.util.ArrayList;
-import java.util.Iterator;
import java.util.List;
/**
@@ -151,12 +150,13 @@ public class RxTaskPool {
public void stop() {
running = false;
synchronized (mutex) {
- Iterator<AbstractRxTask> i = idle.iterator();
- while (i.hasNext()) {
- AbstractRxTask worker = i.next();
- returnWorker(worker);
- i.remove();
+ for (AbstractRxTask worker : idle) {
+ worker.close();
}
+ idle.clear();
+ // Wake up any thread parked in getRxTask() so that it sees that
+ // running is false and returns rather than waiting forever
+ mutex.notifyAll();
}
}
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java b/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
index 37efaae4d5..c14f754ff0 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReplicationTask.java
@@ -44,8 +44,7 @@ import org.apache.juli.logging.LogFactory;
/**
* A worker thread class which drains a channel and processes received messages.
* Each instance services a single SelectionKey per invocation. After completion,
- * the worker returns itself to its parent pool for reuse, unless the key is null
- * in which case it returns early without doing so.
+ * the worker returns itself to its parent pool for reuse.
*/
public class NioReplicationTask extends AbstractRxTask {
@@ -89,61 +88,66 @@ public class NioReplicationTask extends AbstractRxTask {
// loop forever waiting for work to do
@Override
public synchronized void run() {
- if (buffer == null) {
- int size = getRxBufSize();
- if (key.channel() instanceof DatagramChannel) {
- size = ChannelReceiver.MAX_UDP_SIZE;
- }
- if ((getOptions() & OPTION_DIRECT_BUFFER) == OPTION_DIRECT_BUFFER) {
- buffer = ByteBuffer.allocateDirect(size);
- } else {
- buffer = ByteBuffer.allocate(size);
- }
- } else {
- buffer.clear();
- }
- if (key == null) {
- return; // just in case
- }
- if (log.isTraceEnabled()) {
- log.trace("Servicing key:" + key);
- }
-
try {
- ObjectReader reader = (ObjectReader) key.attachment();
- if (reader == null) {
- if (log.isTraceEnabled()) {
- log.trace("No object reader, cancelling:" + key);
+ if (buffer == null) {
+ int size = getRxBufSize();
+ if (key.channel() instanceof DatagramChannel) {
+ size = ChannelReceiver.MAX_UDP_SIZE;
}
- cancelKey(key);
- } else {
- if (log.isTraceEnabled()) {
- log.trace("Draining channel:" + key);
+ if ((getOptions() & OPTION_DIRECT_BUFFER) == OPTION_DIRECT_BUFFER) {
+ buffer = ByteBuffer.allocateDirect(size);
+ } else {
+ buffer = ByteBuffer.allocate(size);
}
-
- drainChannel(key, reader);
+ } else {
+ buffer.clear();
}
- } catch (Exception e) {
- // this is common, since the sockets on the other
- // end expire after a certain time.
- if (e instanceof CancelledKeyException) {
- // do nothing
- } else if (e instanceof IOException) {
- // don't spew out stack traces for IO exceptions unless debug is enabled.
- if (log.isDebugEnabled()) {
- log.debug(sm.getString("nioReplicationTask.unable.drainChannel.ioe", e.getMessage()), e);
+ if (key == null) {
+ return; // just in case
+ }
+ if (log.isTraceEnabled()) {
+ log.trace("Servicing key:" + key);
+ }
+
+ try {
+ ObjectReader reader = (ObjectReader) key.attachment();
+ if (reader == null) {
+ if (log.isTraceEnabled()) {
+ log.trace("No object reader, cancelling:" + key);
+ }
+ cancelKey(key);
} else {
- log.warn(sm.getString("nioReplicationTask.unable.drainChannel.ioe", e.getMessage()));
+ if (log.isTraceEnabled()) {
+ log.trace("Draining channel:" + key);
+ }
+
+ drainChannel(key, reader);
}
- } else if (log.isErrorEnabled()) {
- // this is a real error, log it.
- log.error(sm.getString("nioReplicationTask.exception.drainChannel"), e);
+ } catch (Exception e) {
+ // this is common, since the sockets on the other
+ // end expire after a certain time.
+ if (e instanceof CancelledKeyException) {
+ // do nothing
+ } else if (e instanceof IOException) {
+ // don't spew out stack traces for IO exceptions unless debug is enabled.
+ if (log.isDebugEnabled()) {
+ log.debug(sm.getString("nioReplicationTask.unable.drainChannel.ioe", e.getMessage()), e);
+ } else {
+ log.warn(sm.getString("nioReplicationTask.unable.drainChannel.ioe", e.getMessage()));
+ }
+ } else if (log.isErrorEnabled()) {
+ // this is a real error, log it.
+ log.error(sm.getString("nioReplicationTask.exception.drainChannel"), e);
+ }
+ cancelKey(key);
}
- cancelKey(key);
+ } finally {
+ key = null;
+ // done, ready for more, return to pool -- done from a finally
+ // block so the worker can never leak out of the pool, not even
+ // when an Error escapes the catch block above
+ getTaskPool().returnWorker(this);
}
- key = null;
- // done, ready for more, return to pool
- getTaskPool().returnWorker(this);
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 10/14: Harden RateLimiterBase.destroy() against a limiter whose configuration failed
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit fe8a1dfa49d57bf524b8763d5d77da3c4e09bb0a
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 22:02:23 2026 +0200
Harden RateLimiterBase.destroy() against a limiter whose configuration failed
destroy() dereferenced bucketCounter unconditionally. That field is
only assigned once newCounterInstance() returns, so a limiter whose
setFilterConfig() failed part-way - for example a custom RateLimiter
whose counter constructor rejected the configured duration, or an
embedded caller driving the public RateLimiter API directly with an
invalid duration - would throw a NullPointerException from destroy()
instead of simply doing nothing, masking the original configuration
failure.
Skip the counter teardown when it was never created. The shutdown of
internalExecutorService is a separate, already-guarded block and it
still runs, which matters because the executor is assigned before
the counter, so a failed setFilterConfig() that had created the
internal executor still gets it shut down. Note that the standard
filter lifecycle does not reach this case: an ApplicationFilterConfig
whose init() threw is never registered, so the container never
destroys that filter. This change hardens the public API for direct
and embedded use.
---
java/org/apache/catalina/util/RateLimiterBase.java | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/java/org/apache/catalina/util/RateLimiterBase.java b/java/org/apache/catalina/util/RateLimiterBase.java
index 7108d4ec5b..d34cab576f 100644
--- a/java/org/apache/catalina/util/RateLimiterBase.java
+++ b/java/org/apache/catalina/util/RateLimiterBase.java
@@ -118,7 +118,11 @@ public abstract class RateLimiterBase implements RateLimiter {
@Override
public void destroy() {
- bucketCounter.destroy();
+ // bucketCounter remains null if setFilterConfig() failed before it was assigned, e.g. a counter
+ // constructor rejected the configured duration. Do not mask that failure with an NPE here.
+ if (bucketCounter != null) {
+ bucketCounter.destroy();
+ }
if (internalExecutorService != null) {
try {
internalExecutorService.shutdown();
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 06/14: Stop the listener thread from reviving the listening flag of a stopped receiver
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit df2ba6d60826baa6d1bc576652daf5ef7f204ef5
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 17:57:12 2026 +0200
Stop the listener thread from reviving the listening flag of a stopped
receiver
The listener thread of the NioReceiver set the listening flag
unconditionally, then looked at the selector and, finding it already
closed and nulled by a stop that had completed before the thread got
scheduled, skipped the select loop and returned. The flag stayed set
without anything listening, so every later start exited at the already
started guard and the node silently stopped receiving cluster traffic.
A closely related interleaving, in which the thread set the flag while
stop was waiting for it, made the thread select on the closed selector
it held as a local copy and spin forever at full CPU.
The thread now only sets the flag when the selector is still there to
select on and clears it on exit, an exceptional one, only when it had
set it itself, so a stale thread of a previous start can neither clear
the flag of a live listener nor leave a stale one behind. The loop
condition re-reads the selector field, so a stop that closes and nulls
the selector ends the loop even if the flag had been re-set meanwhile.
The cleanup at the end of the select now also runs when the thread
never got to select, and it no longer assumes the server channel to be
present, which closes the bound but never registered datagram channel
of a stop that raced with the start.
Add TestNioReceiver covering the flag behavior after a stop, the
stale-thread case, the normal stop of a running listener and the
datagram channel cleanup.
---
.../catalina/tribes/transport/nio/NioReceiver.java | 154 +++++++++++++--------
.../tribes/transport/nio/TestNioReceiver.java | 124 +++++++++++++++++
2 files changed, 218 insertions(+), 60 deletions(-)
diff --git a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
index 80cd5ad0ed..260748e1b1 100644
--- a/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
+++ b/java/org/apache/catalina/tribes/transport/nio/NioReceiver.java
@@ -304,80 +304,114 @@ public class NioReceiver extends ReceiverBase implements Runnable, NioReceiverMB
protected void listen() throws Exception {
if (isListening()) {
log.warn(sm.getString("nioReceiver.alreadyStarted"));
+ // The flag belongs to the listener that is running, do not clear
+ // it here
return;
}
- setListen(true);
-
// Avoid NPEs if selector is set to null on stop.
Selector selector = this.selector.get();
- if (selector != null && datagramChannel != null) {
- ObjectReader oreader = new ObjectReader(MAX_UDP_SIZE); // max size for a datagram packet
- registerChannel(selector, datagramChannel, SelectionKey.OP_READ, oreader);
+ // Set the flag only if this thread gets to select. A receiver that
+ // was stopped before this thread got here must not end up marked as
+ // listening, so that a later start is not blocked by the "already
+ // started" guard above.
+ boolean ownsListenFlag = selector != null;
+ if (ownsListenFlag) {
+ setListen(true);
}
- while (isListening() && selector != null) {
- // this may block for a long time, upon return the
- // selected set contains keys of the ready channels
- try {
- events();
- socketTimeouts();
- int n = selector.select(getSelectorTimeout());
- if (n == 0) {
- // there is a good chance that we got here
- // because the TcpReplicationThread called
- // selector wakeup().
- // if that happens, we must ensure that that
- // thread has enough time to call interestOps
- // synchronized (interestOpsMutex) {
- // if we got the lock, means there are no
- // keys trying to register for the
- // interestOps method
- // }
- continue; // nothing to do
- }
- // get an iterator over the set of selected keys
- Iterator<SelectionKey> it = selector.selectedKeys().iterator();
- // look at each key in the selected set
- while (it.hasNext()) {
- SelectionKey key = it.next();
- // Is a new connection coming in?
- if (key.isAcceptable()) {
- ServerSocketChannel server = (ServerSocketChannel) key.channel();
- SocketChannel channel = server.accept();
- channel.socket().setReceiveBufferSize(getRxBufSize());
- channel.socket().setSendBufferSize(getTxBufSize());
- channel.socket().setTcpNoDelay(getTcpNoDelay());
- channel.socket().setKeepAlive(getSoKeepAlive());
- channel.socket().setOOBInline(getOoBInline());
- channel.socket().setReuseAddress(getSoReuseAddress());
- channel.socket().setSoLinger(getSoLingerOn(), getSoLingerTime());
- channel.socket().setSoTimeout(getTimeout());
- Object attach = new ObjectReader(channel);
- registerChannel(selector, channel, SelectionKey.OP_READ, attach);
+ try {
+ if (ownsListenFlag && datagramChannel != null) {
+ ObjectReader oreader = new ObjectReader(MAX_UDP_SIZE); // max size for a datagram packet
+ registerChannel(selector, datagramChannel, SelectionKey.OP_READ, oreader);
+ }
+
+ // Test the field, not only the local reference, so that a stop
+ // which closes and nulls the selector while this thread is in the
+ // select also ends this loop even if the thread had re-set the
+ // listening flag in the meantime
+ while (ownsListenFlag && isListening() && this.selector.get() != null) {
+ // this may block for a long time, upon return the
+ // selected set contains keys of the ready channels
+ try {
+ events();
+ socketTimeouts();
+ int n = selector.select(getSelectorTimeout());
+ if (n == 0) {
+ // there is a good chance that we got here
+ // because the TcpReplicationThread called
+ // selector wakeup().
+ // if that happens, we must ensure that that
+ // thread has enough time to call interestOps
+ // synchronized (interestOpsMutex) {
+ // if we got the lock, means there are no
+ // keys trying to register for the
+ // interestOps method
+ // }
+ continue; // nothing to do
}
- // is there data to read on this channel?
- if (key.isReadable()) {
- readDataFromSocket(key);
- } else {
- key.interestOps(key.interestOps() & (~SelectionKey.OP_WRITE));
+ // get an iterator over the set of selected keys
+ Iterator<SelectionKey> it = selector.selectedKeys().iterator();
+ // look at each key in the selected set
+ while (it.hasNext()) {
+ SelectionKey key = it.next();
+ // Is a new connection coming in?
+ if (key.isAcceptable()) {
+ ServerSocketChannel server = (ServerSocketChannel) key.channel();
+ SocketChannel channel = server.accept();
+ try {
+ channel.socket().setReceiveBufferSize(getRxBufSize());
+ channel.socket().setSendBufferSize(getTxBufSize());
+ channel.socket().setTcpNoDelay(getTcpNoDelay());
+ channel.socket().setKeepAlive(getSoKeepAlive());
+ channel.socket().setOOBInline(getOoBInline());
+ channel.socket().setReuseAddress(getSoReuseAddress());
+ channel.socket().setSoLinger(getSoLingerOn(), getSoLingerTime());
+ channel.socket().setSoTimeout(getTimeout());
+ Object attach = new ObjectReader(channel);
+ registerChannel(selector, channel, SelectionKey.OP_READ, attach);
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ try {
+ channel.close();
+ } catch (IOException ioe) {
+ t.addSuppressed(ioe);
+ }
+ throw t;
+ }
+ }
+ // is there data to read on this channel?
+ if (key.isReadable()) {
+ readDataFromSocket(key);
+ } else {
+ key.interestOps(key.interestOps() & (~SelectionKey.OP_WRITE));
+ }
+
+ // remove key from selected set, it's been handled
+ it.remove();
}
-
- // remove key from selected set, it's been handled
- it.remove();
+ } catch (ClosedSelectorException cse) {
+ // ignore is normal at shutdown or stop listen socket
+ } catch (CancelledKeyException nx) {
+ log.warn(sm.getString("nioReceiver.clientDisconnect"));
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
+ log.error(sm.getString("nioReceiver.requestError"), t);
}
- } catch (ClosedSelectorException cse) {
- // ignore is normal at shutdown or stop listen socket
- } catch (CancelledKeyException nx) {
- log.warn(sm.getString("nioReceiver.clientDisconnect"));
- } catch (Throwable t) {
- ExceptionUtils.handleThrowable(t);
- log.error(sm.getString("nioReceiver.requestError"), t);
+
}
+ } finally {
+ if (ownsListenFlag) {
+ // This thread set the flag itself, so it clears it on every
+ // exit path, an exceptional exit included
+ setListen(false);
+ }
+ }
+ if (serverChannel != null) {
+ serverChannel.close();
}
- serverChannel.close();
if (datagramChannel != null) {
try {
datagramChannel.close();
diff --git a/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java b/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
new file mode 100644
index 0000000000..89e9360819
--- /dev/null
+++ b/test/org/apache/catalina/tribes/transport/nio/TestNioReceiver.java
@@ -0,0 +1,124 @@
+/*
+ * 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.catalina.tribes.transport.nio;
+
+import java.lang.reflect.Field;
+import java.nio.channels.DatagramChannel;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.catalina.tribes.group.GroupChannel;
+
+public class TestNioReceiver {
+
+ /*
+ * A stop that raced with the start of the listener thread used to leave
+ * the listening flag set, because the thread re-set it after the stop had
+ * cleared it. Every later start then exited at the "already started"
+ * guard and the receiver never listened again. The thread now only sets
+ * the flag when the selector is still there to select on.
+ */
+ @Test
+ public void testListenWithANullSelectorDoesNotSetTheFlag() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.listen();
+ Assert.assertFalse(receiver.isListening());
+ }
+
+ /*
+ * A stale listener thread of a previous start that hits the "already
+ * started" guard must leave the flag of the live listener alone, so that
+ * it cannot tear a running receiver down.
+ */
+ @Test
+ public void testStaleThreadDoesNotClearTheFlagOfALiveListener() {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setListen(true);
+ receiver.run();
+ Assert.assertTrue(receiver.isListening());
+ }
+
+ /*
+ * The headline race: the receiver is fully stopped before the listener
+ * thread gets to run. The thread must then not set the listening flag, so
+ * the receiver is not left marked as listening without anything selecting,
+ * which would block every later start at the "already started" guard.
+ */
+ @Test
+ public void testListenAfterStopDoesNotSetTheFlag() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress("127.0.0.1");
+ receiver.setPort(0);
+ receiver.bind();
+ receiver.stopListening();
+ receiver.listen();
+ Assert.assertFalse(receiver.isListening());
+ }
+
+ /*
+ * The thread that set the listening flag itself clears it again when the
+ * selector is closed and nulled underneath it by a stop.
+ */
+ @Test
+ public void testListenerThreadClearsTheFlagOnStop() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress("127.0.0.1");
+ receiver.setPort(0);
+ receiver.bind();
+ Thread thread = new Thread(receiver);
+ thread.start();
+ long deadline = System.currentTimeMillis() + 10000;
+ while (!receiver.isListening() && System.currentTimeMillis() < deadline) {
+ Thread.sleep(50);
+ }
+ Assert.assertTrue("the listener thread never started listening", receiver.isListening());
+ receiver.stopListening();
+ thread.join(10000);
+ Assert.assertFalse("the listener thread did not stop", thread.isAlive());
+ Assert.assertFalse(receiver.isListening());
+ }
+
+ /*
+ * The datagram channel is bound by bind() but only registered with the
+ * selector in listen(), so stopListening() cannot reach it. The listener
+ * thread that runs after the stop must still close it during its cleanup,
+ * even when it never got to select.
+ */
+ @Test
+ public void testListenAfterStopClosesTheBoundDatagramChannel() throws Exception {
+ NioReceiver receiver = new NioReceiver();
+ receiver.setChannel(new GroupChannel());
+ receiver.setAddress("127.0.0.1");
+ receiver.setPort(0);
+ receiver.setUdpPort(45678);
+ receiver.bind();
+ Field field = NioReceiver.class.getDeclaredField("datagramChannel");
+ field.setAccessible(true);
+ DatagramChannel datagramChannel = (DatagramChannel) field.get(receiver);
+ Assert.assertNotNull("bind() did not open the datagram channel", datagramChannel);
+ receiver.stopListening();
+ receiver.listen();
+ Assert.assertFalse("the datagram channel was not closed", datagramChannel.isOpen());
+ Assert.assertNull("the datagram channel reference was not cleared", field.get(receiver));
+ Assert.assertFalse(receiver.isListening());
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
(tomcat) 13/14: Guard the time bucket counter maintenance futures against a racing destroy()
Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rmaucher pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 6aca9be31d1c1a56d1f211a3c3ec3353acccf08b
Author: opencode <re...@apache.org>
AuthorDate: Sun Oct 11 22:38:37 2026 +0200
Guard the time bucket counter maintenance futures against a racing destroy()
The maintenanceFuture field of TimeBucketCounterBase was written by
the MaintenanceMonitor task on the executor thread and read,
cancelled and nulled by destroy() on a container thread without any
happens-before edge between the two. Within that window the monitor
could observe the stale, already cancelled future, treat it as a
failed task, schedule a replacement periodic Maintenance task and
publish it after destroy() had nulled the field. The replacement was
then never cancelled: on a shared executor it kept running for the
lifetime of the executor, retaining the destroyed counter and its
bucket map.
Introduce a destroyed flag and use the object monitor to guard it
together with both future fields. destroy() sets the flag, clears
the map and cancels the futures as one critical section, and the
monitor returns immediately when the flag is set, so a task can no
longer be scheduled after the counter has been destroyed. The
scheduling in the constructor also happens under the monitor now,
making the guarded invariant hold for every access; the object
escapes to the executor at that point. destroy() becomes documented
as thread safe and idempotent.
---
.../catalina/util/TimeBucketCounterBase.java | 59 ++++++++++++++--------
1 file changed, 39 insertions(+), 20 deletions(-)
diff --git a/java/org/apache/catalina/util/TimeBucketCounterBase.java b/java/org/apache/catalina/util/TimeBucketCounterBase.java
index 7aec8de153..85c5d04738 100644
--- a/java/org/apache/catalina/util/TimeBucketCounterBase.java
+++ b/java/org/apache/catalina/util/TimeBucketCounterBase.java
@@ -43,9 +43,12 @@ public abstract class TimeBucketCounterBase {
// Map to hold the buckets
private final ConcurrentHashMap<String,AtomicInteger> map = new ConcurrentHashMap<>();
- // The future allowing control of the background processor.
+ // The futures controlling the background tasks, plus the destroyed flag. All three are guarded by this
+ // object's monitor, which is held by destroy() on a container thread and by MaintenanceMonitor on the
+ // executor thread.
private ScheduledFuture<?> maintenanceFuture;
private ScheduledFuture<?> monitorFuture;
+ private boolean destroyed;
private final ScheduledExecutorService executorService;
private final long sleeptime;
private final int bucketDuration;
@@ -73,7 +76,12 @@ public abstract class TimeBucketCounterBase {
// Start our thread
if (sleeptime > 0) {
- monitorFuture = executorService.scheduleWithFixedDelay(new MaintenanceMonitor(), 0, 60, TimeUnit.SECONDS);
+ // Synchronize, since this is the one write not performed under the monitor by the maintenance or
+ // destroy() threads, and the object escapes to the executor while the constructor is still running
+ synchronized (this) {
+ monitorFuture = executorService.scheduleWithFixedDelay(new MaintenanceMonitor(), 0, 60,
+ TimeUnit.SECONDS);
+ }
}
}
@@ -167,17 +175,21 @@ public abstract class TimeBucketCounterBase {
/**
- * Stops threads created by this object and cleans up resources.
+ * Stops threads created by this object and cleans up resources. This method is thread safe and idempotent. On
+ * return, no maintenance or monitor task remains scheduled.
*/
public void destroy() {
- map.clear();
- if (monitorFuture != null) {
- monitorFuture.cancel(true);
- monitorFuture = null;
- }
- if (maintenanceFuture != null) {
- maintenanceFuture.cancel(true);
- maintenanceFuture = null;
+ synchronized (this) {
+ destroyed = true;
+ map.clear();
+ if (monitorFuture != null) {
+ monitorFuture.cancel(true);
+ monitorFuture = null;
+ }
+ if (maintenanceFuture != null) {
+ maintenanceFuture.cancel(true);
+ maintenanceFuture = null;
+ }
}
}
@@ -211,17 +223,24 @@ public abstract class TimeBucketCounterBase {
private class MaintenanceMonitor implements Runnable {
@Override
public void run() {
- if (sleeptime > 0 && (maintenanceFuture == null || maintenanceFuture.isDone())) {
- if (maintenanceFuture != null && maintenanceFuture.isDone()) {
- // There was an error executing the scheduled task, get it and log it
- try {
- maintenanceFuture.get();
- } catch (InterruptedException | ExecutionException e) {
- log.error(sm.getString("timebucket.maintenance.error"), e);
+ synchronized (TimeBucketCounterBase.this) {
+ if (destroyed) {
+ // destroy() has run. Do not schedule any replacement task, or the new future would never be
+ // cancelled and the destroyed counter would be retained by the executor for its lifetime.
+ return;
+ }
+ if (sleeptime > 0 && (maintenanceFuture == null || maintenanceFuture.isDone())) {
+ if (maintenanceFuture != null && maintenanceFuture.isDone()) {
+ // There was an error executing the scheduled task, get it and log it
+ try {
+ maintenanceFuture.get();
+ } catch (InterruptedException | ExecutionException e) {
+ log.error(sm.getString("timebucket.maintenance.error"), e);
+ }
}
+ maintenanceFuture = executorService.scheduleWithFixedDelay(new Maintenance(), sleeptime,
+ sleeptime, TimeUnit.MILLISECONDS);
}
- maintenanceFuture = executorService.scheduleWithFixedDelay(new Maintenance(), sleeptime, sleeptime,
- TimeUnit.MILLISECONDS);
}
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org