Fixed a slew of shutdown exceptions that arose due to improper or out-of-order resource clean-up, e.g. listeners, timers, executors.
Change-Id: I37c351c4202b32e92c076d9d566b96d7ff8d313a
diff --git a/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java b/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
index b851776..2397c64 100644
--- a/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
+++ b/core/net/src/main/java/org/onosproject/cfg/impl/ComponentConfigManager.java
@@ -114,11 +114,8 @@
String componentName = componentClass.getName();
checkNotNull(componentName, COMPONENT_NULL);
Map<String, ConfigProperty> cps = properties.remove(componentName);
- if (cps != null) {
+ if (clear && cps != null) {
cps.keySet().forEach(name -> store.unsetProperty(componentName, name));
- }
-
- if (clear) {
clearExistingValues(componentName);
}
}
diff --git a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
index f4b82d9..4ae3872 100644
--- a/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
+++ b/core/net/src/main/java/org/onosproject/net/proxyarp/impl/ProxyArpManager.java
@@ -95,11 +95,11 @@
@Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
protected DeviceService deviceService;
- private final Multimap<Device, PortNumber> internalPorts =
- HashMultimap.<Device, PortNumber>create();
+ private final Multimap<Device, PortNumber> internalPorts = HashMultimap.create();
+ private final Multimap<Device, PortNumber> externalPorts = HashMultimap.create();
- private final Multimap<Device, PortNumber> externalPorts =
- HashMultimap.<Device, PortNumber>create();
+ private final DeviceListener deviceListener = new InternalDeviceListener();
+ private final InternalLinkListener linkListener = new InternalLinkListener();
/**
* Listens to both device service and link service to determine
@@ -107,16 +107,17 @@
*/
@Activate
public void activate() {
- deviceService.addListener(new InternalDeviceListener());
- linkService.addListener(new InternalLinkListener());
+ deviceService.addListener(deviceListener);
+ linkService.addListener(linkListener);
determinePortLocations();
-
log.info("Started");
}
@Deactivate
public void deactivate() {
+ deviceService.removeListener(deviceListener);
+ linkService.removeListener(linkListener);
log.info("Stopped");
}
diff --git a/openflow/api/src/main/java/org/onosproject/openflow/controller/driver/AbstractOpenFlowSwitch.java b/openflow/api/src/main/java/org/onosproject/openflow/controller/driver/AbstractOpenFlowSwitch.java
index 65a8c55..50c0351 100644
--- a/openflow/api/src/main/java/org/onosproject/openflow/controller/driver/AbstractOpenFlowSwitch.java
+++ b/openflow/api/src/main/java/org/onosproject/openflow/controller/driver/AbstractOpenFlowSwitch.java
@@ -96,14 +96,14 @@
@Override
public final void sendMsg(OFMessage m) {
- if (role == RoleState.MASTER) {
+ if (role == RoleState.MASTER && channel.isWritable()) {
channel.write(Collections.singletonList(m));
}
}
@Override
public final void sendMsg(List<OFMessage> msgs) {
- if (role == RoleState.MASTER) {
+ if (role == RoleState.MASTER && channel.isWritable()) {
channel.write(msgs);
}
}
diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
index 8db294b..3920e2d 100644
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LLDPLinkProvider.java
@@ -73,6 +73,8 @@
@Component(immediate = true)
public class LLDPLinkProvider extends AbstractProvider implements LinkProvider {
+ private static final String PROVIDER_NAME = "org.onosproject.provider.lldp";
+
private static final String PROP_USE_BDDP = "useBDDP";
private static final String PROP_DISABLE_LD = "disableLinkDiscovery";
private static final String PROP_LLDP_SUPPRESSION = "lldpSuppression";
@@ -132,13 +134,13 @@
* Creates an OpenFlow link provider.
*/
public LLDPLinkProvider() {
- super(new ProviderId("lldp", "org.onosproject.provider.lldp"));
+ super(new ProviderId("lldp", PROVIDER_NAME));
}
@Activate
public void activate(ComponentContext context) {
cfgService.registerProperties(getClass());
- appId = coreService.registerApplication("org.onosproject.provider.lldp");
+ appId = coreService.registerApplication(PROVIDER_NAME);
// to load configuration at startup
modified(context);
@@ -188,14 +190,14 @@
if (disableLinkDiscovery) {
return;
}
- executor.shutdownNow();
- for (LinkDiscovery ld : discoverers.values()) {
- ld.stop();
- }
providerRegistry.unregister(this);
deviceService.removeListener(listener);
packetService.removeProcessor(listener);
masterService.removeListener(roleListener);
+
+ executor.shutdownNow();
+ discoverers.values().forEach(LinkDiscovery::stop);
+ discoverers.clear();
providerService = null;
log.info("Stopped");
diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
index 005bd5c..6f3ffdc 100644
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LinkDiscovery.java
@@ -104,8 +104,8 @@
this.pktService = pktService;
this.mastershipService = checkNotNull(masterService, "WTF!");
- this.slowPorts = Collections.synchronizedSet(new HashSet<Long>());
- this.fastPorts = Collections.synchronizedSet(new HashSet<Long>());
+ this.slowPorts = Collections.synchronizedSet(new HashSet<>());
+ this.fastPorts = Collections.synchronizedSet(new HashSet<>());
this.portProbeCount = new HashMap<>();
this.lldpPacket = new ONOSLLDP();
this.lldpPacket.setChassisId(device.chassisId());
@@ -296,14 +296,14 @@
}
public synchronized void stop() {
- timeout.cancel();
isStopped = true;
+ timeout.cancel();
}
public synchronized void start() {
if (isStopped) {
- timeout = Timer.getTimer().newTimeout(this, 0, MILLISECONDS);
isStopped = false;
+ timeout = Timer.getTimer().newTimeout(this, 0, MILLISECONDS);
} else {
log.warn("LinkDiscovery started multiple times?");
}
@@ -361,8 +361,8 @@
return slowPorts.contains(portNumber) || fastPorts.contains(portNumber);
}
- public boolean isStopped() {
- return isStopped;
+ public synchronized boolean isStopped() {
+ return isStopped || timeout.isCancelled();
}
}
diff --git a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
index eb72230..c0dc9f8 100644
--- a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
+++ b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/OpenFlowDeviceProvider.java
@@ -268,8 +268,8 @@
providerService.deviceConnected(did, description);
providerService.updatePorts(did, buildPortDescriptions(sw));
- PortStatsCollector psc = new PortStatsCollector(
- controller.getSwitch(dpid), POLL_INTERVAL);
+ PortStatsCollector psc =
+ new PortStatsCollector(controller.getSwitch(dpid), POLL_INTERVAL);
psc.start();
collectors.put(dpid, psc);
}
@@ -314,7 +314,7 @@
/**
* Translates a RoleState to the corresponding MastershipRole.
*
- * @param response
+ * @param response role state
* @return a MastershipRole
*/
private MastershipRole roleOf(RoleState response) {
@@ -334,7 +334,6 @@
/**
* Builds a list of port descriptions for a given list of ports.
*
- * @param ports the list of ports
* @return list of portdescriptions
*/
private List<PortDescription> buildPortDescriptions(OpenFlowSwitch sw) {
diff --git a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/PortStatsCollector.java b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/PortStatsCollector.java
index 36d7948..c872a82 100644
--- a/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/PortStatsCollector.java
+++ b/providers/openflow/device/src/main/java/org/onosproject/provider/of/device/impl/PortStatsCollector.java
@@ -45,8 +45,7 @@
private final AtomicLong xidAtomic = new AtomicLong(1);
private Timeout timeout;
-
- private boolean stopTimer = false;
+ private volatile boolean stopped;
/**
* Creates a GroupStatsCollector object.
@@ -60,23 +59,22 @@
}
@Override
- public void run(Timeout timeout) throws Exception {
+ public void run(Timeout to) throws Exception {
+ if (stopped || timeout.isCancelled()) {
+ return;
+ }
log.trace("Collecting stats for {}", sw.getStringId());
sendPortStatistic();
- if (!this.stopTimer) {
+ if (!stopped && !timeout.isCancelled()) {
log.trace("Scheduling stats collection in {} seconds for {}",
this.refreshInterval, this.sw.getStringId());
- timeout.getTimer().newTimeout(this, refreshInterval,
- TimeUnit.SECONDS);
+ timeout.getTimer().newTimeout(this, refreshInterval, TimeUnit.SECONDS);
}
}
private void sendPortStatistic() {
- if (log.isTraceEnabled()) {
- log.trace("sendGroupStatistics {}:{}", sw.getStringId(), sw.getRole());
- }
if (sw.getRole() != RoleState.MASTER) {
return;
}
@@ -91,17 +89,18 @@
/**
* Starts the collector.
*/
- public void start() {
+ public synchronized void start() {
log.info("Starting Port Stats collection thread for {}", sw.getStringId());
+ stopped = false;
timeout = timer.newTimeout(this, 1, TimeUnit.SECONDS);
}
/**
* Stops the collector.
*/
- public void stop() {
+ public synchronized void stop() {
log.info("Stopping Port Stats collection thread for {}", sw.getStringId());
- this.stopTimer = true;
+ stopped = true;
timeout.cancel();
}
}
diff --git a/utils/netty/src/main/java/org/onlab/netty/NettyMessagingManager.java b/utils/netty/src/main/java/org/onlab/netty/NettyMessagingManager.java
index 8d567eb..0bbb8c1 100644
--- a/utils/netty/src/main/java/org/onlab/netty/NettyMessagingManager.java
+++ b/utils/netty/src/main/java/org/onlab/netty/NettyMessagingManager.java
@@ -43,6 +43,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.Executor;
+import java.util.concurrent.RejectedExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicLong;
@@ -301,7 +302,11 @@
@Override
protected void channelRead0(ChannelHandlerContext ctx, InternalMessage message) throws Exception {
- dispatchLocally(message);
+ try {
+ dispatchLocally(message);
+ } catch (RejectedExecutionException e) {
+ log.warn("Unable to dispatch message due to {}", e.getMessage());
+ }
}
@Override
diff --git a/web/gui/src/main/java/org/onosproject/ui/impl/UiWebSocket.java b/web/gui/src/main/java/org/onosproject/ui/impl/UiWebSocket.java
index c571f8f..958ad32 100644
--- a/web/gui/src/main/java/org/onosproject/ui/impl/UiWebSocket.java
+++ b/web/gui/src/main/java/org/onosproject/ui/impl/UiWebSocket.java
@@ -20,6 +20,7 @@
import com.fasterxml.jackson.databind.node.ObjectNode;
import org.eclipse.jetty.websocket.WebSocket;
import org.onlab.osgi.ServiceDirectory;
+import org.onlab.osgi.ServiceNotFoundException;
import org.onosproject.cluster.ClusterService;
import org.onosproject.cluster.ControllerNode;
import org.onosproject.ui.UiConnection;
@@ -100,11 +101,19 @@
@Override
public void onOpen(Connection connection) {
- log.info("GUI client connected");
this.connection = connection;
this.control = (FrameConnection) connection;
- createHandlers();
- sendInstanceData();
+ try {
+ createHandlers();
+ sendInstanceData();
+ log.info("GUI client connected");
+
+ } catch (ServiceNotFoundException e) {
+ log.warn("Unable to open GUI connection; services have been shut-down");
+ this.connection.close();
+ this.connection = null;
+ this.control = null;
+ }
}
@Override