You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shenyu.apache.org by li...@apache.org on 2026/09/02 04:22:21 UTC
(shenyu) branch master updated: fix: avoid concurrent modification in base data cache (#7019)
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 <50...@users.noreply.github.com>
AuthorDate: Wed Sep 2 12:22:16 2026 +0800
fix: avoid concurrent modification in base data cache (#7019)
Co-authored-by: aias00 <li...@apache.org>
---
.../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