Copilot commented on code in PR #7062:
URL: https://github.com/apache/shenyu/pull/7062#discussion_r4032734051
##########
shenyu-loadbalancer/src/main/java/org/apache/shenyu/loadbalancer/cache/UpstreamCheckTask.java:
##########
@@ -286,7 +287,7 @@ public void triggerRemoveOne(final String selectorId, final
Upstream upstream) {
*/
public void putToMap(final Map<String, List<Upstream>> map, final String
selectorId, final Upstream upstream) {
synchronized (lock) {
- List<Upstream> list = MapUtils.computeIfAbsent(map, selectorId, k
-> Lists.newArrayList());
+ List<Upstream> list = MapUtils.computeIfAbsent(map, selectorId, k
-> new CopyOnWriteArrayList<>());
if (!list.contains(upstream)) {
list.add(upstream);
Review Comment:
`CopyOnWriteArrayList` only makes individual operations safe; it does not
make the existing `size()`/`get(i)` traversals atomic. For example,
`RandomLoadBalancer` captures `size()` and then indexes the list while a
health-check removal can occur, so `get(i)` can still throw
`IndexOutOfBoundsException` (the same pattern exists in
`ShortestResponseLoadBalancer`). Return a per-request snapshot or update the
index-based load balancers to iterate over a stable snapshot before relying on
this change.
--
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]
