Fix the wrong use of notify and wait. (#798)

The previous code was using low-level synchronization based on
notify and wait - and it was wrong. The code allowed for a race condition
where notify is called before wait. In that case, notify will have no effect,
and wait will wait forever until interrupted by the test timeout.
I believe this to be the root cause of #793 (and the previous occurrences
in #540).

Just avoid the low-level synchronization and use Semaphore instead.

1 files changed, 7 insertions, 21 deletions

diff --git a/contrib/agent/src/integration-test/java/io/opencensus/contrib/agent/instrumentation/ExecutorInstrumentationIT.java b/contrib/agent/src/integration-test/java/io/opencensus/contrib/agent/instrumentation/ExecutorInstrumentationIT.java
index 8f0cedd0..7cab5590 100644
--- a/contrib/agent/src/integration-test/java/io/opencensus/contrib/agent/instrumentation/ExecutorInstrumentationIT.java
+++ b/contrib/agent/src/integration-test/java/io/opencensus/contrib/agent/instrumentation/ExecutorInstrumentationIT.java
@@ -23,6 +23,7 @@ import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
+import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.After;
 import org.junit.Before;
@@ -62,7 +63,7 @@ public class ExecutorInstrumentationIT {
     final Context context = Context.current().withValue(KEY, "myvalue");
     previousContext = context.attach();
 
-    final AtomicBoolean tested = new AtomicBoolean(false);
+    final Semaphore tested = new Semaphore(0);
 
     executor.execute(
         new Runnable() {
@@ -71,19 +72,11 @@ public class ExecutorInstrumentationIT {
             assertThat(Thread.currentThread()).isNotSameAs(callerThread);
             assertThat(Context.current()).isSameAs(context);
             assertThat(KEY.get()).isEqualTo("myvalue");
-            tested.set(true);
-
-            synchronized (tested) {
-              tested.notify();
-            }
+            tested.release();
           }
         });
 
-    synchronized (tested) {
-      tested.wait();
-    }
-
-    assertThat(tested.get()).isTrue();
+    tested.acquire();
   }
 
   @Test(timeout = 60000)
@@ -169,7 +162,7 @@ public class ExecutorInstrumentationIT {
     final Context context = Context.current().withValue(KEY, "myvalue");
     previousContext = context.attach();
 
-    final AtomicBoolean tested = new AtomicBoolean(false);
+    final Semaphore tested = new Semaphore(0);
 
     Context.currentContextExecutor(executor)
         .execute(
@@ -192,18 +185,11 @@ public class ExecutorInstrumentationIT {
                 assertThat(Thread.currentThread()).isNotSameAs(callerThread);
                 assertThat(Context.current()).isSameAs(context);
                 assertThat(KEY.get()).isEqualTo("myvalue");
-                tested.set(true);
 
-                synchronized (tested) {
-                  tested.notify();
-                }
+                tested.release();
               }
             });
 
-    synchronized (tested) {
-      tested.wait();
-    }
-
-    assertThat(tested.get()).isTrue();
+    tested.acquire();
   }
 }