PDGGK opened a new issue, #39319:
URL: https://github.com/apache/shardingsphere/issues/39319
## Bug Report
**For English only**, other languages will not accept.
### Which version of ShardingSphere did you use?
master (latest)
### Which project did you use? ShardingSphere-JDBC or ShardingSphere-Proxy?
Both — `RoundRobinLoadBalanceAlgorithm` lives in `infra` and is the default
`ROUND_ROBIN` load balancer used by readwrite-splitting.
### Expected behavior
`RoundRobinLoadBalanceAlgorithm.getTargetName()` keeps returning a valid
target name for the lifetime of the process.
### Actual behavior
Once the internal counter passes `Integer.MAX_VALUE`, every call throws
`ArrayIndexOutOfBoundsException` for any replica group whose size is not a
power of two.
```
java.lang.ArrayIndexOutOfBoundsException: Index -2 out of bounds for length 3
at
RoundRobinLoadBalanceAlgorithm.getTargetName(RoundRobinLoadBalanceAlgorithm.java:36)
```
### Reason analyze (If you can)
`RoundRobinLoadBalanceAlgorithm.java:36`:
```java
return availableTargetNames.get(Math.abs(count.getAndIncrement()) %
availableTargetNames.size());
```
`Math.abs(Integer.MIN_VALUE)` is `Integer.MIN_VALUE` — `abs` cannot
represent `2^31` in an `int`, so it returns the negative value unchanged. The
remainder is then negative and `List.get` throws:
| group size | `Math.abs(Integer.MIN_VALUE) % size` | |
| --- | --- | --- |
| 2 | 0 | ok |
| 3 | **-2** | throws |
| 4 | 0 | ok |
| 5 | **-3** | throws |
| 8 | 0 | ok |
Power-of-two group sizes happen to yield 0 and survive, which is why a
2-replica setup never shows it. `getTargetName` is annotated
`@HighFrequencyInvocation` and the counter is never reset, so a long-lived
proxy handling read traffic reaches the wrap on its own.
There is prior art for this exact failure: #1265 reported the same
`ArrayIndexOutOfBoundsException` in 2018 and was closed as completed. The fix
at the time was a `count.compareAndSet(readDataSourceNames.size(), 0)` line
that kept the counter bounded. That line was later removed in #17422 while the
counter was being changed from a static map to an instance field, and the
`Math.abs(...)` expression was kept. (For the record, that old line was itself
racy under concurrency — several threads could step past `size` before any of
them observed it — so the point is not that a good guard was lost, but that the
current code has no bound on the counter at all.)
### Steps to reproduce the behavior
Set the counter to `Integer.MAX_VALUE` and call the algorithm twice against
a 3-element list:
```java
LoadBalanceAlgorithm algorithm =
TypedSPILoader.getService(LoadBalanceAlgorithm.class, "ROUND_ROBIN", new
Properties());
Plugins.getMemberAccessor().set(
RoundRobinLoadBalanceAlgorithm.class.getDeclaredField("count"),
algorithm, new AtomicInteger(Integer.MAX_VALUE));
List<String> availableTargetNames = Arrays.asList("ds_1", "ds_2", "ds_3");
algorithm.getTargetName("ds", availableTargetNames); // ok, index 1
algorithm.getTargetName("ds", availableTargetNames); //
ArrayIndexOutOfBoundsException: Index -2 out of bounds for length 3
```
### Example codes for reproduce this issue (such as a github link)
The snippet above runs as-is inside `RoundRobinLoadBalanceAlgorithmTest`. I
have a fix and a regression test ready and will open a PR referencing this
issue.
`Math.floorMod(count.getAndIncrement(), availableTargetNames.size())` is a
one-line replacement that is identical to the current expression for every
non-negative counter value — I verified that exhaustively for all counters in
`[0, 5_000_000]` across group sizes 1..16 — and returns a valid index after the
wrap instead of throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail:
[email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
