Sho SHIMIZU
Committed by Gerrit Code Review

Make ObjectiveInstaller thread safe

Objective installer may have been dispatched to a different thread and
numAttempts was possibly accessed from multiple threads

This solution is to create a new instance for the next trial

Change-Id: I5d85f87567241f3e072c38f094eb5c7ba511a6a5
...@@ -58,6 +58,7 @@ import java.util.Map; ...@@ -58,6 +58,7 @@ import java.util.Map;
58 import java.util.Set; 58 import java.util.Set;
59 import java.util.concurrent.ExecutorService; 59 import java.util.concurrent.ExecutorService;
60 60
61 +import static com.google.common.base.Preconditions.checkNotNull;
61 import static java.util.concurrent.Executors.newFixedThreadPool; 62 import static java.util.concurrent.Executors.newFixedThreadPool;
62 import static org.onlab.util.Tools.groupedThreads; 63 import static org.onlab.util.Tools.groupedThreads;
63 import static org.onosproject.security.AppGuard.checkPermission; 64 import static org.onosproject.security.AppGuard.checkPermission;
...@@ -150,18 +151,21 @@ public class FlowObjectiveManager implements FlowObjectiveService { ...@@ -150,18 +151,21 @@ public class FlowObjectiveManager implements FlowObjectiveService {
150 private final DeviceId deviceId; 151 private final DeviceId deviceId;
151 private final Objective objective; 152 private final Objective objective;
152 153
153 - private int numAttempts = 0; 154 + private final int numAttempts;
154 155
155 public ObjectiveInstaller(DeviceId deviceId, Objective objective) { 156 public ObjectiveInstaller(DeviceId deviceId, Objective objective) {
156 - this.deviceId = deviceId; 157 + this(deviceId, objective, 1);
157 - this.objective = objective; 158 + }
159 +
160 + public ObjectiveInstaller(DeviceId deviceId, Objective objective, int attemps) {
161 + this.deviceId = checkNotNull(deviceId);
162 + this.objective = checkNotNull(objective);
163 + this.numAttempts = checkNotNull(attemps);
158 } 164 }
159 165
160 @Override 166 @Override
161 public void run() { 167 public void run() {
162 try { 168 try {
163 - numAttempts++;
164 -
165 Pipeliner pipeliner = getDevicePipeliner(deviceId); 169 Pipeliner pipeliner = getDevicePipeliner(deviceId);
166 170
167 if (pipeliner != null) { 171 if (pipeliner != null) {
...@@ -174,7 +178,7 @@ public class FlowObjectiveManager implements FlowObjectiveService { ...@@ -174,7 +178,7 @@ public class FlowObjectiveManager implements FlowObjectiveService {
174 } 178 }
175 } else if (numAttempts < INSTALL_RETRY_ATTEMPTS) { 179 } else if (numAttempts < INSTALL_RETRY_ATTEMPTS) {
176 Thread.sleep(INSTALL_RETRY_INTERVAL); 180 Thread.sleep(INSTALL_RETRY_INTERVAL);
177 - executorService.submit(this); 181 + executorService.submit(new ObjectiveInstaller(deviceId, objective, numAttempts + 1));
178 } else { 182 } else {
179 // Otherwise we've tried a few times and failed, report an 183 // Otherwise we've tried a few times and failed, report an
180 // error back to the user. 184 // error back to the user.
......