Fix ML_SYNC_ON_FIELD_TO_GUARD_CHANGING_THAT_FIELD issue
Change-Id: I367514c698e825ccf6fda63104e2a51f51c190c4
diff --git a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
index 7e8cb4c..8b447af 100644
--- a/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
+++ b/apps/segmentrouting/src/main/java/org/onosproject/segmentrouting/DefaultRoutingHandler.java
@@ -32,6 +32,8 @@
import java.util.HashSet;
import java.util.List;
import java.util.Set;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import static com.google.common.base.Preconditions.checkNotNull;
@@ -45,7 +47,8 @@
private HashMap<DeviceId, ECMPShortestPathGraph> currentEcmpSpgMap;
private HashMap<DeviceId, ECMPShortestPathGraph> updatedEcmpSpgMap;
private DeviceConfiguration config;
- private Status populationStatus;
+ private final Lock statusLock = new ReentrantLock();
+ private volatile Status populationStatus;
/**
* Represents the default routing population status.
@@ -86,35 +89,40 @@
*/
public boolean populateAllRoutingRules() {
- populationStatus = Status.STARTED;
- rulePopulator.resetCounter();
- log.info("Starts to populate routing rules");
- log.debug("populateAllRoutingRules: populationStatus is STARTED");
+ statusLock.lock();
+ try {
+ populationStatus = Status.STARTED;
+ rulePopulator.resetCounter();
+ log.info("Starts to populate routing rules");
+ log.debug("populateAllRoutingRules: populationStatus is STARTED");
- for (Device sw : srManager.deviceService.getDevices()) {
- if (srManager.mastershipService.getLocalRole(sw.id()) != MastershipRole.MASTER) {
- log.debug("populateAllRoutingRules: skipping device {}...we are not master",
- sw.id());
- continue;
+ for (Device sw : srManager.deviceService.getDevices()) {
+ if (srManager.mastershipService.getLocalRole(sw.id()) != MastershipRole.MASTER) {
+ log.debug("populateAllRoutingRules: skipping device {}...we are not master",
+ sw.id());
+ continue;
+ }
+
+ ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(sw.id(), srManager);
+ if (!populateEcmpRoutingRules(sw.id(), ecmpSpg)) {
+ log.debug("populateAllRoutingRules: populationStatus is ABORTED");
+ populationStatus = Status.ABORTED;
+ log.debug("Abort routing rule population");
+ return false;
+ }
+ currentEcmpSpgMap.put(sw.id(), ecmpSpg);
+
+ // TODO: Set adjacency routing rule for all switches
}
- ECMPShortestPathGraph ecmpSpg = new ECMPShortestPathGraph(sw.id(), srManager);
- if (!populateEcmpRoutingRules(sw.id(), ecmpSpg)) {
- log.debug("populateAllRoutingRules: populationStatus is ABORTED");
- populationStatus = Status.ABORTED;
- log.debug("Abort routing rule population");
- return false;
- }
- currentEcmpSpgMap.put(sw.id(), ecmpSpg);
-
- // TODO: Set adjacency routing rule for all switches
+ log.debug("populateAllRoutingRules: populationStatus is SUCCEEDED");
+ populationStatus = Status.SUCCEEDED;
+ log.info("Completes routing rule population. Total # of rules pushed : {}",
+ rulePopulator.getCounter());
+ return true;
+ } finally {
+ statusLock.unlock();
}
-
- log.debug("populateAllRoutingRules: populationStatus is SUCCEEDED");
- populationStatus = Status.SUCCEEDED;
- log.info("Completes routing rule population. Total # of rules pushed : {}",
- rulePopulator.getCounter());
- return true;
}
/**
@@ -127,7 +135,8 @@
*/
public boolean populateRoutingRulesForLinkStatusChange(Link linkFail) {
- synchronized (populationStatus) {
+ statusLock.lock();
+ try {
if (populationStatus == Status.STARTED) {
log.warn("Previous rule population is not finished.");
@@ -179,6 +188,8 @@
log.warn("Failed to repopulate the rules.");
return false;
}
+ } finally {
+ statusLock.unlock();
}
}
@@ -504,7 +515,8 @@
* ABORTED status when any groups required for flows is not set yet.
*/
public void startPopulationProcess() {
- synchronized (populationStatus) {
+ statusLock.lock();
+ try {
if (populationStatus == Status.IDLE
|| populationStatus == Status.SUCCEEDED
|| populationStatus == Status.ABORTED) {
@@ -514,6 +526,8 @@
log.warn("Not initiating startPopulationProcess as populationStatus is {}",
populationStatus);
}
+ } finally {
+ statusLock.unlock();
}
}
@@ -522,13 +536,16 @@
* Mostly the process is aborted when the groups required are not set yet.
*/
public void resumePopulationProcess() {
- synchronized (populationStatus) {
+ statusLock.lock();
+ try {
if (populationStatus == Status.ABORTED) {
populationStatus = Status.STARTED;
// TODO: we need to restart from the point aborted instead of
// restarting.
populateAllRoutingRules();
}
+ } finally {
+ statusLock.unlock();
}
}
}