Merge pull request #1294 from miguelaferreira/nsx-execution-counter-per-thread

Implement a NSX API request execution counter per threadThe NSX plugin has a execution counter to prevent infinite recursion (and as a result a stack overflow exception). However, the thread safeness of this counter are not as desired. The counter was implemented with an AtomicInteger which make it safe for multiple threads to update and read it. The desired property would be to have a counter per thread.

This PR addresses that issue.

* pr/1294:
  Fix execution counter to support separate counts per thread
  Add test to check that each thread has it's own execution counter

Signed-off-by: Remi Bergsma <github@remi.nl>

plugins/network-elements/nicira-nvp/src/main/java/com/cloud/network/nicira/ExecutionCounter.java

@@ -19,15 +19,19 @@
 
 package com.cloud.network.nicira;
 
-import java.util.concurrent.atomic.AtomicInteger;
-
 public class ExecutionCounter {
 
     private final int executionLimit;
-    private final AtomicInteger executionCount = new AtomicInteger(0);
+    private final ThreadLocal<Integer> executionCount;
 
     public ExecutionCounter(final int executionLimit) {
         this.executionLimit = executionLimit;
+        executionCount = new ThreadLocal<Integer>() {
+            @Override
+            protected Integer initialValue() {
+                return new Integer(0);
+            }
+        };
     }
 
     public ExecutionCounter resetExecutionCounter() {
@@ -40,7 +44,7 @@ public class ExecutionCounter {
     }
 
     public ExecutionCounter incrementExecutionCounter() {
-        executionCount.incrementAndGet();
+        executionCount.set(executionCount.get() + 1);
         return this;
     }
 

plugins/network-elements/nicira-nvp/src/test/java/com/cloud/network/nicira/ExecutionCounterTest.java

@@ -22,6 +22,11 @@ package com.cloud.network.nicira;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
 
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicInteger;
+
 import org.junit.Test;
 
 public class ExecutionCounterTest {
@@ -52,4 +57,46 @@ public class ExecutionCounterTest {
 
         assertThat(executionCounter.hasReachedExecutionLimit(), equalTo(true));
     }
+
+    @Test
+    public void testConcurrentUpdatesToCounter() throws Exception {
+        final ExecutionCounter executionCounter = new ExecutionCounter(0);
+        final ExecutorService executorService = Executors.newFixedThreadPool(3);
+        final AtomicInteger counterTask1 = new AtomicInteger(-1);
+        final AtomicInteger counterTask2 = new AtomicInteger(-1);
+        final AtomicInteger counterTask3 = new AtomicInteger(-1);
+
+        final Runnable task1 = new Runnable() {
+            @Override
+            public void run() {
+                executionCounter.incrementExecutionCounter().incrementExecutionCounter();
+                executionCounter.incrementExecutionCounter().incrementExecutionCounter();
+                counterTask1.set(executionCounter.getValue());
+            }
+        };
+        final Runnable task2 = new Runnable() {
+            @Override
+            public void run() {
+                executionCounter.incrementExecutionCounter().incrementExecutionCounter();
+                counterTask2.set(executionCounter.getValue());
+            }
+        };
+        final Runnable task3 = new Runnable() {
+            @Override
+            public void run() {
+                counterTask3.set(executionCounter.getValue());
+            }
+        };
+
+        executorService.execute(task1);
+        executorService.execute(task2);
+        executorService.execute(task3);
+
+        executorService.shutdown();
+        executorService.awaitTermination(5L, TimeUnit.SECONDS);
+
+        assertThat(counterTask1.get(), equalTo(4));
+        assertThat(counterTask2.get(), equalTo(2));
+        assertThat(counterTask3.get(), equalTo(0));
+    }
 }