Merge pull request #1588 from zliebowitz/fix-stringlocker-behavior

Fix StringLocker behavior.

Author: lesserwhirls

cdm/core/src/main/java/ucar/nc2/StringLocker.java

@@ -1,8 +1,7 @@
 package ucar.nc2;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
+import java.util.*;
+import java.util.concurrent.ConcurrentSkipListSet;
 
 /**
  * A list of strings that only allows one thread to use any given value at the same time.
@@ -13,32 +12,29 @@
 @Deprecated
 public class StringLocker {
 
-  private List<String> stringList = Collections.synchronizedList(new ArrayList<>());
-  private boolean waiting;
+  private final Set<String> stringSet = new ConcurrentSkipListSet<>();
 
   public synchronized void control(String item) {
     // If the string is in use by another thread then wait() for the other thread
-    waiting = stringList.contains(item);
-    while (waiting) {
+    while (stringSet.contains(item)) {
       try {
         wait();
       } catch (InterruptedException e) {
         Thread.currentThread().interrupt();
       }
     }
     // Finished waiting so the thread can have the string
-    stringList.add(item);
+    stringSet.add(item);
   }
 
   public synchronized void release(String item) {
     // Tell StringLocker the thread is done with the string
-    stringList.remove(item);
-    waiting = false;
+    stringSet.remove(item);
     notifyAll();
   }
 
   public String toString() {
-    return stringList.toString();
+    return stringSet.toString();
   }
 
 }

cdm/core/src/test/java/ucar/nc2/StringLockerTest.java

@@ -0,0 +1,37 @@
+package ucar.nc2;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import java.util.concurrent.atomic.AtomicReference;
+
+public class StringLockerTest {
+  @Test
+  public void testNonconflicting() throws InterruptedException {
+    Thread t = new Thread(() -> {
+      StringLocker locker = new StringLocker();
+      locker.control("first");
+      locker.control("second");
+      locker.release("first");
+      locker.release("second");
+    });
+    t.start();
+    t.join(500);
+    Assert.assertFalse(t.isAlive());
+  }
+
+
+  @Test
+  public void testNonconflictingOtherOrder() throws InterruptedException {
+    Thread t = new Thread(() -> {
+      StringLocker locker = new StringLocker();
+      locker.control("first");
+      locker.control("second");
+      locker.release("second");
+      locker.release("first");
+    });
+    t.start();
+    t.join(500);
+    Assert.assertFalse(t.isAlive());
+  }
+}