From 36b162cc70b7c77525b6d3effbdc5efbdcdc2f7b Mon Sep 17 00:00:00 2001
From: Jerry Chin <metrxqin@gmail.com>
Date: Tue, 27 Apr 2021 19:33:58 +0800
Subject: [PATCH] Apply volatile modifier for fields in FlowRuleManager while
 keep concurrency semantics intact (#2107)

- Revert #1783
---
 .../slots/block/flow/FlowRuleManager.java     | 35 +++++++++----------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java
index 9dfa3f27d0..10d34c1dc1 100755
--- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java
+++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slots/block/flow/FlowRuleManager.java
@@ -16,14 +16,12 @@
 package com.alibaba.csp.sentinel.slots.block.flow;
 
 import java.util.ArrayList;
-import java.util.Collections;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicReference;
 
 import com.alibaba.csp.sentinel.concurrent.NamedThreadFactory;
 import com.alibaba.csp.sentinel.config.SentinelConfig;
@@ -50,7 +48,7 @@
  */
 public class FlowRuleManager {
 
-    private static final AtomicReference<Map<String, List<FlowRule>>> flowRules = new AtomicReference<Map<String, List<FlowRule>>>();
+    private static volatile Map<String, List<FlowRule>> flowRules = new HashMap<>();
 
     private static final FlowPropertyListener LISTENER = new FlowPropertyListener();
     private static SentinelProperty<List<FlowRule>> currentProperty = new DynamicSentinelProperty<List<FlowRule>>();
@@ -60,11 +58,10 @@ public class FlowRuleManager {
         new NamedThreadFactory("sentinel-metrics-record-task", true));
 
     static {
-        flowRules.set(Collections.<String, List<FlowRule>>emptyMap());
         currentProperty.addListener(LISTENER);
         startMetricTimerListener();
     }
-    
+
     /**
      * <p> Start the MetricTimerListener
      * <ol>
@@ -79,12 +76,12 @@ private static void startMetricTimerListener() {
         if (flushInterval <= 0) {
             RecordLog.info("[FlowRuleManager] The MetricTimerListener isn't started. If you want to start it, "
                     + "please change the value(current: {}) of config({}) more than 0 to start it.", flushInterval,
-                    SentinelConfig.METRIC_FLUSH_INTERVAL);
+                SentinelConfig.METRIC_FLUSH_INTERVAL);
             return;
         }
         SCHEDULER.scheduleAtFixedRate(new MetricTimerListener(), 0, flushInterval, TimeUnit.SECONDS);
     }
-    
+
     /**
      * Listen to the {@link SentinelProperty} for {@link FlowRule}s. The property is the source of {@link FlowRule}s.
      * Flow rules can also be set by {@link #loadRules(List)} directly.
@@ -108,7 +105,7 @@ public static void register2Property(SentinelProperty<List<FlowRule>> property)
      */
     public static List<FlowRule> getRules() {
         List<FlowRule> rules = new ArrayList<FlowRule>();
-        for (Map.Entry<String, List<FlowRule>> entry : flowRules.get().entrySet()) {
+        for (Map.Entry<String, List<FlowRule>> entry : flowRules.entrySet()) {
             rules.addAll(entry.getValue());
         }
         return rules;
@@ -124,11 +121,11 @@ public static void loadRules(List<FlowRule> rules) {
     }
 
     static Map<String, List<FlowRule>> getFlowRuleMap() {
-        return flowRules.get();
+        return flowRules;
     }
 
     public static boolean hasConfig(String resource) {
-        return flowRules.get().containsKey(resource);
+        return flowRules.containsKey(resource);
     }
 
     public static boolean isOtherOrigin(String origin, String resourceName) {
@@ -136,7 +133,7 @@ public static boolean isOtherOrigin(String origin, String resourceName) {
             return false;
         }
 
-        List<FlowRule> rules = flowRules.get().get(resourceName);
+        List<FlowRule> rules = flowRules.get(resourceName);
 
         if (rules != null) {
             for (FlowRule rule : rules) {
@@ -152,18 +149,20 @@ public static boolean isOtherOrigin(String origin, String resourceName) {
     private static final class FlowPropertyListener implements PropertyListener<List<FlowRule>> {
 
         @Override
-        public void configUpdate(List<FlowRule> value) {
+        public synchronized void configUpdate(List<FlowRule> value) {
             Map<String, List<FlowRule>> rules = FlowRuleUtil.buildFlowRuleMap(value);
-            //the rules was always not null, it's no need to check nullable
-            //remove checking to avoid IDE warning
-            flowRules.set(rules);
+            if (rules != null) {
+                flowRules = rules;
+            }
             RecordLog.info("[FlowRuleManager] Flow rules received: {}", rules);
         }
 
         @Override
-        public void configLoad(List<FlowRule> conf) {
+        public synchronized void configLoad(List<FlowRule> conf) {
             Map<String, List<FlowRule>> rules = FlowRuleUtil.buildFlowRuleMap(conf);
-            flowRules.set(rules);
+            if (rules != null) {
+                flowRules = rules;
+            }
             RecordLog.info("[FlowRuleManager] Flow rules loaded: {}", rules);
         }
     }