Copilot commented on code in PR #7053:
URL: https://github.com/apache/shenyu/pull/7053#discussion_r4032746884
##########
shenyu-admin/src/main/java/org/apache/shenyu/admin/service/impl/UpstreamCheckService.java:
##########
@@ -398,7 +397,7 @@ private void updateHandler(final String selectorId, final
List<CommonUpstream> u
}
removePendingSync(successList);
if (!successList.isEmpty()) {
- UPSTREAM_MAP.put(selectorId, successList);
+ UPSTREAM_MAP.put(selectorId, toThreadSafeList(successList));
Review Comment:
This conversion prevents iterator failures, but it does not make the
health-check result update atomic. `check` builds `successList` from a snapshot
of `upstreamList`; a concurrent `submitJust` or `replace` can add/replace
entries before this callback runs, and this `put` then overwrites those newer
entries with the stale `successList`. Serialize updates per selector or merge
with the current map value before replacing it so registrations are not lost.
##########
shenyu-admin/src/test/java/org/apache/shenyu/admin/service/UpstreamCheckServiceTest.java:
##########
@@ -259,8 +259,10 @@ public void testFetchUpstreamData() {
upstreamCheckService.fetchUpstreamData();
assertTrue(upstreamMap.containsKey(MOCK_SELECTOR_NAME));
assertEquals(2, upstreamMap.get(MOCK_SELECTOR_NAME).size());
+ assertTrue(upstreamMap.get(MOCK_SELECTOR_NAME) instanceof
CopyOnWriteArrayList);
Review Comment:
These assertions only check the concrete list implementation and do not
reproduce the race this bug fix is meant to address. The active tests do not
exercise `scheduled`/`updateHandler` concurrently (the scheduler test is
disabled), so a future change could reintroduce the CME or stale-update loss
while these assertions still pass; add a deterministic regression test that
runs iteration alongside registration/update and verifies both safety and entry
preservation.
--
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]
