#266 Concurrent DockerCmdExecFactory.getDefaultDockerCmdExecFactory fails on reload

Kindrat · Kindrat · commit db9da85d6219 · 2015-07-11T22:00:55.000+03:00
- test &amp; fix
diff --git a/src/main/java/com/github/dockerjava/core/DockerClientBuilder.java b/src/main/java/com/github/dockerjava/core/DockerClientBuilder.java
@@ -1,15 +1,28 @@
 package com.github.dockerjava.core;
 
-import java.util.ServiceLoader;
-
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.DockerCmdExecFactory;
 import com.github.dockerjava.core.DockerClientConfig.DockerClientConfigBuilder;
 
+import java.util.Iterator;
+import java.util.ServiceLoader;
+
 public class DockerClientBuilder {
 
+    private static Class<? extends DockerCmdExecFactory> factoryClass;
     private static ServiceLoader<DockerCmdExecFactory> serviceLoader = ServiceLoader.load(DockerCmdExecFactory.class);
 
+    static {
+        serviceLoader.reload();
+        Iterator<DockerCmdExecFactory> iterator = serviceLoader.iterator();
+        if (!iterator.hasNext()) {
+           throw new RuntimeException("Fatal: Can't find any implementation of '"
+                 + DockerCmdExecFactory.class.getName() + "' in the current classpath.");
+        }
+
+        factoryClass = iterator.next().getClass();
+    }
+
     private DockerClientImpl dockerClient = null;
 
     private DockerCmdExecFactory dockerCmdExecFactory = null;
@@ -35,15 +48,14 @@ public static DockerClientBuilder getInstance(String serverUrl) {
     }
 
     public static DockerCmdExecFactory getDefaultDockerCmdExecFactory() {
-        // clearing the cache is needed because otherwise we will get
-        // the same DockerCmdExecFactory instance each time
-        serviceLoader.reload();
-        if (!serviceLoader.iterator().hasNext()) {
-            throw new RuntimeException("Fatal: Can't find any implementation of '"
-                    + DockerCmdExecFactory.class.getName() + "' in the current classpath.");
-        }
-
-        return serviceLoader.iterator().next();
+       try
+       {
+          return factoryClass.newInstance();
+       }
+       catch (InstantiationException | IllegalAccessException e)
+       {
+          throw new RuntimeException("Fatal: Can't create new instance of '" + factoryClass.getName() + "'");
+       }
     }
 
     public DockerClientBuilder withDockerCmdExecFactory(DockerCmdExecFactory dockerCmdExecFactory) {
diff --git a/src/test/java/com/github/dockerjava/core/DockerClientBuilderTest.java b/src/test/java/com/github/dockerjava/core/DockerClientBuilderTest.java
@@ -0,0 +1,93 @@
+package com.github.dockerjava.core;
+
+import com.github.dockerjava.api.command.DockerCmdExecFactory;
+import org.testng.annotations.Test;
+
+import java.util.*;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+
+public class DockerClientBuilderTest
+{
+   // Amount of instances created in test
+   private static final int AMOUNT = 100;
+
+   @Test
+   public void testConcurrentClientBuilding() throws Exception
+   {
+      // we use it to check instance uniqueness
+      final Set<DockerCmdExecFactory> instances = Collections.synchronizedSet(new HashSet<DockerCmdExecFactory>());
+
+      Runnable runnable = new Runnable()
+      {
+         @Override
+         public void run()
+         {
+            DockerCmdExecFactory factory = DockerClientBuilder.getDefaultDockerCmdExecFactory();
+            // factory created
+            assertNotNull(factory);
+            // and is unique
+            assertFalse(instances.contains(factory));
+            instances.add(factory);
+         }
+      };
+
+      parallel(AMOUNT, runnable);
+      // set contains all required unique instances
+      assertEquals(instances.size(), AMOUNT);
+   }
+
+   public static void parallel(int threads, final Runnable task) throws Exception
+   {
+      final ExceptionListener exceptionListener = new ExceptionListener();
+      Runnable runnable = new Runnable()
+      {
+         @Override
+         public void run()
+         {
+            try
+            {
+               task.run();
+            }
+            catch (Throwable e)
+            {
+               exceptionListener.onException(e);
+            }
+         }
+      };
+
+      List<Thread> threadList = new ArrayList<>(threads);
+      for (int i = 0; i < threads; i++)
+      {
+         Thread thread = new Thread(runnable);
+         thread.start();
+         threadList.add(thread);
+      }
+      for (Thread thread : threadList)
+      {
+         thread.join();
+      }
+      Throwable exception = exceptionListener.getException();
+      if (exception != null)
+      {
+         throw new RuntimeException(exception);
+      }
+   }
+
+   private static class ExceptionListener
+   {
+      private Throwable exception;
+
+      private synchronized void onException(Throwable e)
+      {
+         exception = e;
+      }
+
+      private synchronized Throwable getException()
+      {
+         return exception;
+      }
+   }
+}
