Fixed javadoc warning; added shared executor/timer wrappers to prevent inadverte…

…nt shutdown; added shutdown to CoreManager.deactivate.

Change-Id: I27f31b5d41050d6d87cd6419ab863201c4585843

@@ -64,9 +64,9 @@ public class CoreManager implements CoreService {
     @Reference(cardinality = ReferenceCardinality.MANDATORY_UNARY)
     protected ComponentConfigService cfgService;
 
-    @Property(name = "sharedThreadPoolSize", intValue = SharedExecutors.DEFAULT_THREAD_SIZE,
+    @Property(name = "sharedThreadPoolSize", intValue = SharedExecutors.DEFAULT_POOL_SIZE,
             label = "Configure shared pool maximum size ")
-    private int sharedThreadPoolSize = SharedExecutors.DEFAULT_THREAD_SIZE;
+    private int sharedThreadPoolSize = SharedExecutors.DEFAULT_POOL_SIZE;
 
     @Activate
     public void activate() {
@@ -80,6 +80,7 @@ public class CoreManager implements CoreService {
     @Deactivate
     public void deactivate() {
         cfgService.unregisterProperties(getClass(), false);
+        SharedExecutors.shutdown();
     }
 
     @Override

+/*
+ * Copyright 2015 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onlab.util;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+/**
+ * Executor service wrapper for shared executors with safeguards on shutdown
+ * to prevent inadvertent shutdown.
+ */
+class SharedExecutorService implements ExecutorService {
+
+    private static final String NOT_ALLOWED = "Shutdown of shared executor is not allowed";
+
+    private ExecutorService executor;
+
+    /**
+     * Creates a wrapper for the given executor service.
+     *
+     * @param executor executor service to wrap
+     */
+    SharedExecutorService(ExecutorService executor) {
+        this.executor = executor;
+    }
+
+    /**
+     * Returns the backing executor service.
+     *
+     * @return backing executor service
+     */
+    ExecutorService backingExecutor() {
+        return executor;
+    }
+
+    /**
+     * Swaps the backing executor with a new one and shuts down the old one.
+     *
+     * @param executor new executor service
+     */
+    void setBackingExecutor(ExecutorService executor) {
+        ExecutorService oldExecutor = this.executor;
+        this.executor = executor;
+        oldExecutor.shutdown();
+    }
+
+    @Override
+    public void shutdown() {
+        throw new UnsupportedOperationException(NOT_ALLOWED);
+    }
+
+    @Override
+    public List<Runnable> shutdownNow() {
+        throw new UnsupportedOperationException(NOT_ALLOWED);
+    }
+
+    @Override
+    public boolean isShutdown() {
+        return executor.isShutdown();
+    }
+
+    @Override
+    public boolean isTerminated() {
+        return executor.isTerminated();
+    }
+
+    @Override
+    public boolean awaitTermination(long timeout, TimeUnit unit)
+            throws InterruptedException {
+        return executor.awaitTermination(timeout, unit);
+    }
+
+    @Override
+    public <T> Future<T> submit(Callable<T> task) {
+        return executor.submit(task);
+    }
+
+    @Override
+    public <T> Future<T> submit(Runnable task, T result) {
+        return executor.submit(task, result);
+    }
+
+    @Override
+    public Future<?> submit(Runnable task) {
+        return executor.submit(task);
+    }
+
+    @Override
+    public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks)
+            throws InterruptedException {
+        return executor.invokeAll(tasks);
+    }
+
+    @Override
+    public <T> List<Future<T>> invokeAll(Collection<? extends Callable<T>> tasks,
+                                         long timeout, TimeUnit unit)
+            throws InterruptedException {
+        return executor.invokeAll(tasks, timeout, unit);
+    }
+
+    @Override
+    public <T> T invokeAny(Collection<? extends Callable<T>> tasks)
+            throws InterruptedException, ExecutionException {
+        return executor.invokeAny(tasks);
+    }
+
+    @Override
+    public <T> T invokeAny(Collection<? extends Callable<T>> tasks,
+                           long timeout, TimeUnit unit)
+            throws InterruptedException, ExecutionException, TimeoutException {
+        return invokeAny(tasks, timeout, unit);
+    }
+
+    @Override
+    public void execute(Runnable command) {
+        executor.execute(command);
+    }
+
+}

@@ -18,9 +18,8 @@ package org.onlab.util;
 
 import java.util.Timer;
 import java.util.concurrent.ExecutorService;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static java.util.concurrent.Executors.newFixedThreadPool;
 import static java.util.concurrent.Executors.newSingleThreadExecutor;
 import static org.onlab.util.Tools.groupedThreads;
@@ -36,21 +35,20 @@ import static org.onlab.util.Tools.groupedThreads;
  */
 public final class SharedExecutors {
 
-    private static final Logger log = LoggerFactory.getLogger(SharedExecutors.class);
+    public static final int DEFAULT_POOL_SIZE = 30;
 
-    // TODO: make this configurable via setPoolSize static method
-    public static final int DEFAULT_THREAD_SIZE = 30;
-    private static int poolSize = DEFAULT_THREAD_SIZE;
+    private static SharedExecutorService singleThreadExecutor =
+            new SharedExecutorService(
+                    newSingleThreadExecutor(groupedThreads("onos/shared",
+                                                           "onos-single-executor")));
 
-    private static ExecutorService singleThreadExecutor =
-            newSingleThreadExecutor(groupedThreads("onos/shared",
-                                                   "onos-single-executor"));
+    private static SharedExecutorService poolThreadExecutor =
+            new SharedExecutorService(
+                    newFixedThreadPool(DEFAULT_POOL_SIZE,
+                                       groupedThreads("onos/shared",
+                                                      "onos-pool-executor-%d")));
 
-    private static ExecutorService poolThreadExecutor =
-            newFixedThreadPool(poolSize, groupedThreads("onos/shared",
-                                                               "onos-pool-executor-%d"));
-
-    private static Timer sharedTimer = new Timer("onos-shared-timer");
+    private static SharedTimer sharedTimer = new SharedTimer();
 
     // Ban public construction
     private SharedExecutors() {
@@ -85,17 +83,41 @@ public final class SharedExecutors {
 
     /**
      * Sets the shared thread pool size.
-     * @param poolSize
+     *
+     * @param poolSize new pool size
      */
     public static void setPoolSize(int poolSize) {
-        if (poolSize > 0) {
-            SharedExecutors.poolSize = poolSize;
-            //TODO: wait for execution previous task in the queue .
-            poolThreadExecutor.shutdown();
-            poolThreadExecutor = newFixedThreadPool(poolSize, groupedThreads("onos/shared",
-                    "onos-pool-executor-%d"));
-        } else {
-            log.warn("Shared Pool Size size must be greater than 0");
+        checkArgument(poolSize > 0, "Shared pool size size must be greater than 0");
+        poolThreadExecutor.setBackingExecutor(
+                newFixedThreadPool(poolSize, groupedThreads("onos/shared",
+                                                            "onos-pool-executor-%d")));
+    }
+
+    /**
+     * Shuts down all shared timers and executors and therefore should be
+     * called only by the framework.
+     */
+    public static void shutdown() {
+        sharedTimer.shutdown();
+        singleThreadExecutor.backingExecutor().shutdown();
+        poolThreadExecutor.backingExecutor().shutdown();
+    }
+
+    // Timer extension which does not allow outside cancel method.
+    private static class SharedTimer extends Timer {
+
+        public SharedTimer() {
+            super("onos-shared-timer");
+        }
+
+        @Override
+        public void cancel() {
+            throw new UnsupportedOperationException("Cancel of shared timer is not allowed");
+        }
+
+        private void shutdown() {
+            super.cancel();
         }
     }
+
 }

@@ -58,13 +58,13 @@ public final class SlidingWindowCounter {
 
         // Initialize each item in the list to an AtomicLong of 0
         this.counters = Collections.nCopies(windowSlots, 0)
-                    .stream()
-                    .map(AtomicLong::new)
-                    .collect(Collectors.toCollection(ArrayList::new));
+                .stream()
+                .map(AtomicLong::new)
+                .collect(Collectors.toCollection(ArrayList::new));
 
         background = Executors.newSingleThreadScheduledExecutor();
         background.scheduleWithFixedDelay(this::advanceHead, 0,
-                SLIDE_WINDOW_PERIOD_SECONDS, TimeUnit.SECONDS);
+                                          SLIDE_WINDOW_PERIOD_SECONDS, TimeUnit.SECONDS);
     }
 
     /**