This is an automated email from the ASF dual-hosted git repository.
garydgregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-lang.git
The following commit(s) were added to refs/heads/master by this push:
new 45c80d995 Keep EventCountCircuitBreaker event count from wrapping on
int overflow (#1825)
45c80d995 is described below
commit 45c80d99559017ea6efcb5db6156bb9a5c850d34
Author: Gary Gregory <[email protected]>
AuthorDate: Sun Oct 18 18:33:12 2026 -0400
Keep EventCountCircuitBreaker event count from wrapping on int overflow
(#1825)
Prevent EventCountCircuitBreaker counter overflow
Saturate the event count at Long.MAX_VALUE so rollover cannot cause an
open circuit breaker to close after an overloaded interval. Document
saturation and counter resets.
Add tests for long overflow boundaries, repeated increments after
saturation, recovery after a quiet interval, and Integer.MAX_VALUE as
the opening threshold.
---
src/changes/changes.xml | 1 +
.../lang3/concurrent/EventCountCircuitBreaker.java | 20 ++++--
.../concurrent/EventCountCircuitBreakerTest.java | 72 +++++++++++++++++++---
3 files changed, 78 insertions(+), 15 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 69da672a2..744b29ddc 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -65,6 +65,7 @@ The <action> type attribute can be add,update,fix,remove.
<action type="fix" dev="ggregory" due-to="gaurav kumar
pandey, Gary Gregory">Prevent StringUtils padding from splitting surrogate
pairs (#1813).</action>
<action issue="LANG-441" type="fix" dev="ggregory" due-to="Sebb, Valentin
Rocher, Adarsh Kumar Omrey, Gary Gregory">DateUtils Javadoc does not fully
define round() behavior.</action>
<action issue="LANG-1838" type="fix" dev="ggregory" due-to="Aryan R.
Ganjoo, Gary Gregory">Fix resource paths for unnamed packages (#1824).</action>
+ <action type="fix" dev="ggregory" due-to="alhuda, Gary
Gregory">Keep EventCountCircuitBreaker event count from wrapping on int
overflow (#1825).</action>
<!-- ADD -->
<!-- UPDATE -->
<action type="update" dev="ggregory" due-to="Gary Gregory">Bump
org.apache.commons:commons-parent from 105 to 106.</action>
diff --git
a/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
b/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
index be508d450..02de9f844 100644
---
a/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
+++
b/src/main/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreaker.java
@@ -224,15 +224,21 @@ public long getEventCount() {
}
/**
- * Returns a new instance of {@link CheckIntervalData} with the event
counter
- * incremented by the given delta. If the delta is 0, this object is
returned.
+ * Returns a new instance of {@link CheckIntervalData} with the event
counter incremented by the given delta. If the delta is 0, this object is
+ * returned.
+ * <p>
+ * If the increment would overflow, the event counter saturates at
{@link Long#MAX_VALUE}.
+ * </p>
*
* @param delta The delta
* @return The updated instance
*/
public CheckIntervalData increment(final int delta) {
- return delta == 0 ? this : new CheckIntervalData(getEventCount() +
delta,
- getCheckIntervalStart());
+ if (delta == 0) {
+ return this;
+ }
+ final long nextCount = getEventCount() + delta;
+ return new CheckIntervalData(nextCount < getEventCount() ?
Long.MAX_VALUE : nextCount, getCheckIntervalStart());
}
}
@@ -480,8 +486,10 @@ public boolean incrementAndCheckState() {
* {@inheritDoc}
*
* <p>
- * The event count is a protective counter and only moves toward the
opening threshold:
- * negative increments are rejected.
+ * The event count is a protective counter: negative increments are
rejected, and an increment that would overflow
+ * {@link Long#MAX_VALUE} saturates the counter at {@link Long#MAX_VALUE}
instead of wrapping negative. Saturation prevents
+ * an overloaded interval from causing an automatic close. The counter
resets when a new check interval starts or the
+ * circuit breaker changes state.
* </p>
*
* @throws IllegalArgumentException Thrown if the increment is negative.
diff --git
a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
index 5e2d57441..cafb25fcb 100644
---
a/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
+++
b/src/test/java/org/apache/commons/lang3/concurrent/EventCountCircuitBreakerTest.java
@@ -25,14 +25,19 @@
import java.beans.PropertyChangeEvent;
import java.beans.PropertyChangeListener;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.commons.lang3.AbstractLangTest;
import org.apache.commons.lang3.ArrayUtils;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Test class for {@code EventCountCircuitBreaker}.
@@ -117,6 +122,22 @@ public EventCountCircuitBreakerTestImpl at(final long
time) {
long nanoTime() {
return currentTime;
}
+
+ /**
+ * Sets the event count near the long boundary without executing
billions of increments.
+ *
+ * @param count The event count
+ * @throws ReflectiveOperationException if the interval data cannot be
initialized
+ */
+ @SuppressWarnings("unchecked")
+ private void setEventCount(final long count) throws
ReflectiveOperationException {
+ final Field field =
EventCountCircuitBreaker.class.getDeclaredField("checkIntervalData");
+ field.setAccessible(true);
+ final AtomicReference<Object> intervalData =
(AtomicReference<Object>) field.get(this);
+ final Constructor<?> constructor =
intervalData.get().getClass().getDeclaredConstructor(long.class, long.class);
+ constructor.setAccessible(true);
+ intervalData.set(constructor.newInstance(count, currentTime));
+ }
}
/** Constant for the opening threshold. */
@@ -301,16 +322,24 @@ void testNotClosingWhenEventCountOverflows() {
}
/**
- * Tests that the circuit breaker opens when the event count of the check
interval
- * exceeds the range of an int.
+ * Tests that saturation prevents closing after heavy load and still
allows recovery after a quiet interval.
+ *
+ * @param count The event count before incrementing
+ * @throws ReflectiveOperationException if the interval data cannot be
initialized
*/
- @Test
- void testOpeningWhenEventCountOverflows() {
- final EventCountCircuitBreakerTestImpl breaker = new
EventCountCircuitBreakerTestImpl(OPENING_THRESHOLD, 1,
- TimeUnit.SECONDS, CLOSING_THRESHOLD, 1, TimeUnit.SECONDS);
- assertTrue(breaker.at(1000).incrementAndCheckState(OPENING_THRESHOLD /
2), "Not closed");
-
assertFalse(breaker.at(2000).incrementAndCheckState(Integer.MAX_VALUE), "Not
open");
- assertTrue(breaker.isOpen(), "Not open at end");
+ @ParameterizedTest
+ @ValueSource(longs = {Long.MAX_VALUE - Integer.MAX_VALUE, Long.MAX_VALUE -
Integer.MAX_VALUE + 1, Long.MAX_VALUE - 1, Long.MAX_VALUE})
+ void testNotClosingWhenEventCountSaturates(final long count) throws
ReflectiveOperationException {
+ final EventCountCircuitBreakerTestImpl breaker = new
EventCountCircuitBreakerTestImpl(OPENING_THRESHOLD,
+ 10, TimeUnit.SECONDS, CLOSING_THRESHOLD, 1, TimeUnit.SECONDS);
+ breaker.at(0).open();
+ breaker.setEventCount(count);
+
assertFalse(breaker.at(1000).incrementAndCheckState(Integer.MAX_VALUE), "Not
open");
+ assertFalse(breaker.at(2000).checkState(), "Closed after a zero
increment");
+ assertFalse(breaker.at(3000).incrementAndCheckState(), "Not open after
another event");
+
assertFalse(breaker.at(4000).incrementAndCheckState(Integer.MAX_VALUE), "Not
open after another batch");
+ assertFalse(breaker.at(NANO_FACTOR + 1).checkState(), "Closed after a
saturated interval");
+ assertTrue(breaker.at(2 * NANO_FACTOR + 2).checkState(), "Not closed
after a quiet interval");
}
/**
@@ -358,6 +387,19 @@ void testNow() {
assertTrue(deltaNanos < 100_000, String.format("Delta %,d ns to
current time too large", deltaNanos));
}
+ /**
+ * Tests that the circuit breaker opens when the event count of the check
interval
+ * exceeds the range of an int.
+ */
+ @Test
+ void testOpeningWhenEventCountOverflows() {
+ final EventCountCircuitBreakerTestImpl breaker = new
EventCountCircuitBreakerTestImpl(OPENING_THRESHOLD, 1,
+ TimeUnit.SECONDS, CLOSING_THRESHOLD, 1, TimeUnit.SECONDS);
+ assertTrue(breaker.at(1000).incrementAndCheckState(OPENING_THRESHOLD /
2), "Not closed");
+
assertFalse(breaker.at(2000).incrementAndCheckState(Integer.MAX_VALUE), "Not
open");
+ assertTrue(breaker.isOpen(), "Not open at end");
+ }
+
/**
* Tests that the circuit breaker opens if all conditions are met.
*/
@@ -391,6 +433,18 @@ void testOpeningWhenThresholdReachedThroughBatch() {
assertFalse(breaker.isClosed(), "Closed");
}
+ /**
+ * Tests that the maximum int opening threshold is exceeded without
wrapping the event count.
+ */
+ @Test
+ void testOpeningWithMaximumThreshold() {
+ final EventCountCircuitBreakerTestImpl breaker = new
EventCountCircuitBreakerTestImpl(Integer.MAX_VALUE, 1,
+ TimeUnit.SECONDS, CLOSING_THRESHOLD, 1, TimeUnit.SECONDS);
+ assertTrue(breaker.at(1000).incrementAndCheckState(Integer.MAX_VALUE),
"Opened at the threshold");
+ assertFalse(breaker.at(2000).incrementAndCheckState(), "Not open above
the threshold");
+ assertTrue(breaker.isOpen(), "Not open at end");
+ }
+
/**
* Tests whether an explicit open operation fully initializes the internal
check data
* object. Otherwise, the circuit breaker may close itself directly
afterwards.
