Yaml reader is not thread safe (#3625)

* Yaml reader is not thread safe

* Yaml reader is not thread safe

Author: arnaudboudier-sogelink

core/src/main/java/org/mapfish/print/config/ConfigurationFactory.java

@@ -22,22 +22,25 @@
 public class ConfigurationFactory {
   private static final Logger LOGGER = LoggerFactory.getLogger(ConfigurationFactory.class);
   @Autowired private ConfigurableApplicationContext context;
-  private Yaml yaml;
+  private ThreadLocal<Yaml> yamlThreadLocal;
   private boolean doValidation = true;
 
   /** initialize this factory. Called by spring after construction. */
   @PostConstruct
   public final void init() {
-    MapfishPrintConstructor constructor = new MapfishPrintConstructor(this.context);
-
     LoaderOptions loaderOptions = new LoaderOptions();
     String maxAliases = System.getenv("PRINT_YAML_MAX_ALIASES");
     if (maxAliases != null) {
       loaderOptions.setMaxAliasesForCollections(Integer.parseInt(maxAliases));
     }
     DumperOptions dumperOptions = new DumperOptions();
     Representer representer = new Representer(dumperOptions);
-    this.yaml = new Yaml(constructor, representer, dumperOptions, loaderOptions);
+    this.yamlThreadLocal =
+        ThreadLocal.withInitial(
+            () -> {
+              MapfishPrintConstructor constructor = new MapfishPrintConstructor(this.context);
+              return new Yaml(constructor, representer, dumperOptions, loaderOptions);
+            });
   }
 
   /**
@@ -64,20 +67,23 @@ public final Configuration getConfig(final File configFile, final InputStream co
     configuration.setConfigurationFile(configFile);
     MapfishPrintConstructor.setConfigurationUnderConstruction(configuration);
 
-    final Configuration config =
-        this.yaml.load(new InputStreamReader(configData, StandardCharsets.UTF_8));
-    if (this.doValidation) {
-      final List<Throwable> validate = config.validate();
-      if (!validate.isEmpty()) {
-        StringBuilder errors = new StringBuilder();
-        for (Throwable throwable : validate) {
-          errors.append("\n\t* ").append(throwable.getMessage());
-          LOGGER.error("Configuration Error found", throwable);
+    try (InputStreamReader reader = new InputStreamReader(configData, StandardCharsets.UTF_8)) {
+      final Configuration config = this.yamlThreadLocal.get().load(reader);
+      if (this.doValidation) {
+        final List<Throwable> validate = config.validate();
+        if (!validate.isEmpty()) {
+          StringBuilder errors = new StringBuilder();
+          for (Throwable throwable : validate) {
+            errors.append("\n\t* ").append(throwable.getMessage());
+            LOGGER.error("Configuration Error found", throwable);
+          }
+          throw new Error(errors.toString(), validate.get(0));
         }
-        throw new Error(errors.toString(), validate.get(0));
       }
+      return config;
+    } finally {
+      this.yamlThreadLocal.remove();
     }
-    return config;
   }
 
   /**

core/src/test/java/org/mapfish/print/config/ConfigurationFactoryTest.java

@@ -5,6 +5,11 @@
 import static org.junit.Assert.assertTrue;
 
 import java.io.File;
+import java.util.concurrent.Callable;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 import org.junit.Before;
 import org.junit.Test;
 import org.mapfish.print.AbstractMapfishSpringTest;
@@ -50,6 +55,48 @@ public void testSpringInjection() throws Exception {
     ((ProcessorWithSpringInjection) processor).assertInjected();
   }
 
+  @Test
+  public void testParallelConfigLoading() throws Exception {
+    File configFileThread1 = getFile(ConfigurationFactoryTest.class, "configThread1.yaml");
+    File configFileThread2 = getFile(ConfigurationFactoryTest.class, "configThread2.yaml");
+    ExecutorService executor = Executors.newFixedThreadPool(2);
+    CountDownLatch startLatch = new CountDownLatch(1);
+
+    Callable<Boolean> task1 =
+        () -> {
+          startLatch.await();
+          Configuration config = configurationFactory.getConfig(configFileThread1);
+          if (config.getTemplates() != null && config.getTemplates().containsKey("main")) {
+            return config.getTemplate("main").getAttributes().get("thread1") != null;
+          }
+          return false;
+        };
+
+    Callable<Boolean> task2 =
+        () -> {
+          startLatch.await();
+          Configuration config = configurationFactory.getConfig(configFileThread2);
+          if (config.getTemplates() != null && config.getTemplates().containsKey("main")) {
+            return config.getTemplate("main").getAttributes().get("thread2") != null;
+          }
+          return false;
+        };
+
+    Future<Boolean> thread1Futur = executor.submit(task1);
+    Future<Boolean> thread2Futur = executor.submit(task2);
+
+    // Threads start at the same time
+    startLatch.countDown();
+
+    boolean thread1Result = thread1Futur.get();
+    boolean thread2Result = thread2Futur.get();
+
+    assertTrue("thread 1 configuration loading was not OK", thread1Result);
+    assertTrue("thread 2 configuration loading was not OK", thread2Result);
+
+    executor.shutdown();
+  }
+
   @Test
   public void testConfigurationInjection() throws Exception {
     File configFile =

core/src/test/resources/org/mapfish/print/config/configThread1.yaml

@@ -0,0 +1,5 @@
+templates:
+  main: !template
+    attributes:
+      thread1: !string
+        default: 'thread1'

core/src/test/resources/org/mapfish/print/config/configThread2.yaml

@@ -0,0 +1,5 @@
+templates:
+  main: !template
+    attributes:
+      thread2: !string
+        default: 'thread2'