This is an automated email from the ASF dual-hosted git repository.
Aias00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/shenyu.git
The following commit(s) were added to refs/heads/master by this push:
new e0b5a62b24 fix: avoid concurrent modification in base data cache
(#7019)
e0b5a62b24 is described below
commit e0b5a62b241f5af5673152fd5ef3fda4550497cd
Author: TheoLi0905 <[email protected]>
AuthorDate: Wed Sep 2 12:22:16 2026 +0800
fix: avoid concurrent modification in base data cache (#7019)
Co-authored-by: aias00 <[email protected]>
---
.../shenyu/plugin/base/cache/BaseDataCache.java | 63 +++++++++++-----------
.../plugin/base/cache/BaseDataCacheTest.java | 53 ++++++++++++++++--
.../base/cache/CommonPluginDataSubscriberTest.java | 8 +--
.../web/controller/LocalPluginControllerTest.java | 4 +-
4 files changed, 85 insertions(+), 43 deletions(-)
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
index 89cabc4e90..216bd4dba8 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/main/java/org/apache/shenyu/plugin/base/cache/BaseDataCache.java
@@ -17,14 +17,15 @@
package org.apache.shenyu.plugin.base.cache;
-import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import org.apache.shenyu.common.dto.PluginData;
import org.apache.shenyu.common.dto.RuleData;
import org.apache.shenyu.common.dto.SelectorData;
+import java.util.ArrayList;
import java.util.Comparator;
import java.util.List;
+import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.ConcurrentMap;
import java.util.stream.Collectors;
@@ -132,10 +133,12 @@ public final class BaseDataCache {
*/
public void removeSelectData(final SelectorData selectorData) {
Optional.ofNullable(selectorData).ifPresent(data -> {
- final List<SelectorData> selectorDataList =
SELECTOR_MAP.get(data.getPluginName());
- synchronized (SELECTOR_MAP) {
- Optional.ofNullable(selectorDataList).ifPresent(list ->
list.removeIf(e -> e.getId().equals(data.getId())));
- }
+ SELECTOR_MAP.computeIfPresent(data.getPluginName(), (key, value)
-> {
+ final List<SelectorData> result = value.stream()
+ .filter(selector -> !Objects.equals(selector.getId(),
data.getId()))
+ .collect(Collectors.toList());
+ return result.isEmpty() ? null : List.copyOf(result);
+ });
});
}
@@ -168,7 +171,7 @@ public final class BaseDataCache {
* Obtain selector data list list.
*
* @param pluginName the plugin name
- * @return the list
+ * @return the immutable snapshot, or {@code null} if no selector data
exists
*/
public List<SelectorData> obtainSelectorData(final String pluginName) {
return SELECTOR_MAP.get(pluginName);
@@ -190,10 +193,12 @@ public final class BaseDataCache {
*/
public void removeRuleData(final RuleData ruleData) {
Optional.ofNullable(ruleData).ifPresent(data -> {
- final List<RuleData> ruleDataList =
RULE_MAP.get(data.getSelectorId());
- synchronized (RULE_MAP) {
- Optional.ofNullable(ruleDataList).ifPresent(list ->
list.removeIf(rule -> rule.getId().equals(data.getId())));
- }
+ RULE_MAP.computeIfPresent(data.getSelectorId(), (key, value) -> {
+ final List<RuleData> result = value.stream()
+ .filter(rule -> !Objects.equals(rule.getId(),
data.getId()))
+ .collect(Collectors.toList());
+ return result.isEmpty() ? null : List.copyOf(result);
+ });
});
}
@@ -226,7 +231,7 @@ public final class BaseDataCache {
* Obtain rule data list list.
*
* @param selectorId the selector id
- * @return the list
+ * @return the immutable snapshot, or {@code null} if no rule data exists
*/
public List<RuleData> obtainRuleData(final String selectorId) {
return RULE_MAP.get(selectorId);
@@ -267,17 +272,13 @@ public final class BaseDataCache {
*/
private void ruleAccept(final RuleData data) {
String selectorId = data.getSelectorId();
- synchronized (RULE_MAP) {
- if (RULE_MAP.containsKey(selectorId)) {
- List<RuleData> existList = RULE_MAP.get(selectorId);
- final List<RuleData> resultList = existList.stream().filter(r
-> !r.getId().equals(data.getId())).collect(Collectors.toList());
- resultList.add(data);
- final List<RuleData> collect =
resultList.stream().sorted(Comparator.comparing(RuleData::getSort)).collect(Collectors.toList());
- RULE_MAP.put(selectorId, collect);
- } else {
- RULE_MAP.put(selectorId, Lists.newArrayList(data));
- }
- }
+ RULE_MAP.compute(selectorId, (key, value) -> {
+ final List<RuleData> result = Objects.isNull(value) ? new
ArrayList<>() : new ArrayList<>(value);
+ result.removeIf(rule -> Objects.equals(rule.getId(),
data.getId()));
+ result.add(data);
+ result.sort(Comparator.comparing(RuleData::getSort));
+ return List.copyOf(result);
+ });
}
/**
@@ -287,16 +288,12 @@ public final class BaseDataCache {
*/
private void selectorAccept(final SelectorData data) {
String key = data.getPluginName();
- synchronized (SELECTOR_MAP) {
- if (SELECTOR_MAP.containsKey(key)) {
- List<SelectorData> existList = SELECTOR_MAP.get(key);
- final List<SelectorData> resultList =
existList.stream().filter(r ->
!r.getId().equals(data.getId())).collect(Collectors.toList());
- resultList.add(data);
- final List<SelectorData> collect =
resultList.stream().sorted(Comparator.comparing(SelectorData::getSort)).collect(Collectors.toList());
- SELECTOR_MAP.put(key, collect);
- } else {
- SELECTOR_MAP.put(key, Lists.newArrayList(data));
- }
- }
+ SELECTOR_MAP.compute(key, (pluginName, value) -> {
+ final List<SelectorData> result = Objects.isNull(value) ? new
ArrayList<>() : new ArrayList<>(value);
+ result.removeIf(selector -> Objects.equals(selector.getId(),
data.getId()));
+ result.add(data);
+ result.sort(Comparator.comparing(SelectorData::getSort));
+ return List.copyOf(result);
+ });
}
}
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
index 5f9ed46481..e2c0c5fc5a 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/BaseDataCacheTest.java
@@ -24,12 +24,15 @@ import org.apache.shenyu.common.dto.SelectorData;
import org.junit.jupiter.api.Test;
import java.lang.reflect.Field;
+import java.util.Iterator;
import java.util.List;
import java.util.concurrent.ConcurrentHashMap;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
/**
* Test cases for BaseDataCache.
@@ -142,7 +145,7 @@ public final class BaseDataCacheTest {
selectorMap.put(mockPluginName1, Lists.newArrayList(selectorData));
BaseDataCache.getInstance().removeSelectData(selectorData);
- assertEquals(Lists.newArrayList(), selectorMap.get(mockPluginName1));
+ assertNull(selectorMap.get(mockPluginName1));
}
@Test
@@ -167,7 +170,7 @@ public final class BaseDataCacheTest {
selectorMap.put(mockPluginName2,
Lists.newArrayList(secondCachedSelectorData));
BaseDataCache.getInstance().cleanSelectorDataSelf(Lists.newArrayList(firstCachedSelectorData));
- assertEquals(Lists.newArrayList(), selectorMap.get(mockPluginName1));
+ assertNull(selectorMap.get(mockPluginName1));
assertEquals(Lists.newArrayList(secondCachedSelectorData),
selectorMap.get(mockPluginName2));
}
@@ -181,6 +184,27 @@ public final class BaseDataCacheTest {
assertEquals(Lists.newArrayList(selectorData), selectorDataList);
}
+ @Test
+ public void testSelectorDataSnapshotRemainsStableAfterDelete() {
+ BaseDataCache.getInstance().cleanSelectorData();
+ SelectorData firstSelectorData =
SelectorData.builder().id("1").pluginName(mockPluginName1).sort(1).build();
+ SelectorData secondSelectorData =
SelectorData.builder().id("2").pluginName(mockPluginName1).sort(2).build();
+ BaseDataCache.getInstance().cacheSelectData(firstSelectorData);
+ BaseDataCache.getInstance().cacheSelectData(secondSelectorData);
+
+ List<SelectorData> snapshot =
BaseDataCache.getInstance().obtainSelectorData(mockPluginName1);
+ Iterator<SelectorData> iterator = snapshot.iterator();
+ assertEquals(firstSelectorData, iterator.next());
+
+ BaseDataCache.getInstance().removeSelectData(firstSelectorData);
+
+ assertDoesNotThrow(() -> iterator.forEachRemaining(selector ->
assertNotNull(selector)));
+ assertEquals(Lists.newArrayList(firstSelectorData,
secondSelectorData), snapshot);
+ assertEquals(Lists.newArrayList(secondSelectorData),
BaseDataCache.getInstance().obtainSelectorData(mockPluginName1));
+ assertThrows(UnsupportedOperationException.class, () ->
snapshot.add(firstSelectorData));
+ BaseDataCache.getInstance().cleanSelectorData();
+ }
+
@Test
public void testCacheRuleData() throws NoSuchFieldException,
IllegalAccessException {
RuleData firstCachedRuleData =
RuleData.builder().id("1").selectorId(mockSelectorId1).sort(1).build();
@@ -200,7 +224,7 @@ public final class BaseDataCacheTest {
ruleMap.put(mockSelectorId1, Lists.newArrayList(ruleData));
BaseDataCache.getInstance().removeRuleData(ruleData);
- assertEquals(Lists.newArrayList(), ruleMap.get(mockSelectorId1));
+ assertNull(ruleMap.get(mockSelectorId1));
}
@Test
@@ -225,7 +249,7 @@ public final class BaseDataCacheTest {
ruleMap.put(mockSelectorId2, Lists.newArrayList(secondCachedRuleData));
BaseDataCache.getInstance().cleanRuleDataSelf(Lists.newArrayList(firstCachedRuleData));
- assertEquals(Lists.newArrayList(), ruleMap.get(mockSelectorId1));
+ assertNull(ruleMap.get(mockSelectorId1));
assertEquals(Lists.newArrayList(secondCachedRuleData),
ruleMap.get(mockSelectorId2));
}
@@ -239,6 +263,27 @@ public final class BaseDataCacheTest {
assertEquals(Lists.newArrayList(ruleData), ruleDataList);
}
+ @Test
+ public void testRuleDataSnapshotRemainsStableAfterDelete() {
+ BaseDataCache.getInstance().cleanRuleData();
+ RuleData firstRuleData =
RuleData.builder().id("1").selectorId(mockSelectorId1).sort(1).build();
+ RuleData secondRuleData =
RuleData.builder().id("2").selectorId(mockSelectorId1).sort(2).build();
+ BaseDataCache.getInstance().cacheRuleData(firstRuleData);
+ BaseDataCache.getInstance().cacheRuleData(secondRuleData);
+
+ List<RuleData> snapshot =
BaseDataCache.getInstance().obtainRuleData(mockSelectorId1);
+ Iterator<RuleData> iterator = snapshot.iterator();
+ assertEquals(firstRuleData, iterator.next());
+
+ BaseDataCache.getInstance().removeRuleData(firstRuleData);
+
+ assertDoesNotThrow(() -> iterator.forEachRemaining(rule ->
assertNotNull(rule)));
+ assertEquals(Lists.newArrayList(firstRuleData, secondRuleData),
snapshot);
+ assertEquals(Lists.newArrayList(secondRuleData),
BaseDataCache.getInstance().obtainRuleData(mockSelectorId1));
+ assertThrows(UnsupportedOperationException.class, () ->
snapshot.add(firstRuleData));
+ BaseDataCache.getInstance().cleanRuleData();
+ }
+
@SuppressWarnings("rawtypes")
private ConcurrentHashMap getFieldByName(final String name) throws
NoSuchFieldException, IllegalAccessException {
BaseDataCache baseDataCache = BaseDataCache.getInstance();
diff --git
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
index cd1d9cd1cd..3df6109763 100644
---
a/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
+++
b/shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/cache/CommonPluginDataSubscriberTest.java
@@ -165,7 +165,7 @@ public final class CommonPluginDataSubscriberTest {
commonPluginDataSubscriber.unSelectorSubscribe(selectorData);
- assertEquals(Lists.newArrayList(),
baseDataCache.obtainSelectorData(selectorData.getPluginName()));
+
assertNull(baseDataCache.obtainSelectorData(selectorData.getPluginName()));
assertNull(matchDataCache.obtainSelectorData(mockPluginName1,
path));
assertNull(matchDataCache.obtainRuleData(mockPluginName1, path));
assertNull(matchDataCache.obtainRuleData(mockPluginName1,
emptyRulePath));
@@ -202,7 +202,7 @@ public final class CommonPluginDataSubscriberTest {
assertNotNull(baseDataCache.obtainSelectorData(secondCachedSelectorData.getPluginName()));
commonPluginDataSubscriber.refreshSelectorDataSelf(Lists.newArrayList(firstCachedSelectorData));
- assertEquals(Lists.newArrayList(),
baseDataCache.obtainSelectorData(firstCachedSelectorData.getPluginName()));
+
assertNull(baseDataCache.obtainSelectorData(firstCachedSelectorData.getPluginName()));
assertEquals(Lists.newArrayList(secondCachedSelectorData),
baseDataCache.obtainSelectorData(secondCachedSelectorData.getPluginName()));
}
@@ -224,7 +224,7 @@ public final class CommonPluginDataSubscriberTest {
assertNotNull(baseDataCache.obtainRuleData(ruleData.getSelectorId()));
commonPluginDataSubscriber.unRuleSubscribe(ruleData);
- assertEquals(Lists.newArrayList(),
baseDataCache.obtainRuleData(ruleData.getSelectorId()));
+ assertNull(baseDataCache.obtainRuleData(ruleData.getSelectorId()));
}
@Test
@@ -253,7 +253,7 @@ public final class CommonPluginDataSubscriberTest {
assertNotNull(baseDataCache.obtainRuleData(firstCachedRuleData.getSelectorId()));
commonPluginDataSubscriber.refreshRuleDataSelf(Lists.newArrayList(firstCachedRuleData));
- assertEquals(Lists.newArrayList(),
baseDataCache.obtainRuleData(firstCachedRuleData.getSelectorId()));
+
assertNull(baseDataCache.obtainRuleData(firstCachedRuleData.getSelectorId()));
assertEquals(Lists.newArrayList(secondCachedRuleData),
baseDataCache.obtainRuleData(secondCachedRuleData.getSelectorId()));
}
diff --git
a/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
b/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
index 434eb87d8e..5b99aee226 100644
---
a/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
+++
b/shenyu-web/src/test/java/org/apache/shenyu/web/controller/LocalPluginControllerTest.java
@@ -238,7 +238,7 @@ public final class LocalPluginControllerTest {
.param("id", testSelectorId))
.andExpect(status().isOk())
.andReturn();
-
assertThat(baseDataCache.obtainSelectorData(selectorPluginName)).isEmpty();
+
assertThat(baseDataCache.obtainSelectorData(selectorPluginName)).isNull();
}
@Test
@@ -390,7 +390,7 @@ public final class LocalPluginControllerTest {
.andReturn();
final List<RuleData> selectorId =
baseDataCache.obtainRuleData(testSelectorId);
- Assertions.assertTrue(selectorId.isEmpty());
+ Assertions.assertNull(selectorId);
}
@Test
