Merge pull request #647 from syjer/646-fix-init

move setting of initPending to false only after initializing the jdk log manager, fixes #646
danfickle · Feb 12, 2021 · 67913a4 · 67913a4
2 parents 0130ccf + e06e641
commit 67913a4
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 8 deletions.
diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/JDKXRLogger.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/JDKXRLogger.java
@@ -35,11 +35,11 @@
  * https://github.com/danfickle/openhtmltopdf/wiki/Logging
  */
 public class JDKXRLogger implements XRLogger {
-    private boolean initPending = true;
+    private volatile boolean initPending = true;
 
     // Keep a map of Loggers so they are not garbage collected
     // which makes them lose their settings we have applied.
-    private Map<String, Logger> loggers;
+    private volatile Map<String, Logger> loggers;
 
     private final boolean useParent;
     private final Level level;
@@ -107,10 +107,11 @@ private void init(boolean useParent, Level level, Handler handler, Formatter for
             if (!initPending) {
                 return;
             }
-            //now change this immediately, in case something fails
-            initPending = false;
-
-            initializeJDKLogManager(useParent, level, handler, formatter);
+            try {
+                initializeJDKLogManager(useParent, level, handler, formatter);
+            } finally {
+                initPending = false;
+            }
     }
 
     private void initializeJDKLogManager(boolean useParent, Level level, Handler handler, Formatter formatter) {

diff --git a/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/XRLog.java b/openhtmltopdf-core/src/main/java/com/openhtmltopdf/util/XRLog.java
@@ -52,8 +52,8 @@ private static String registerLoggerByName(final String loggerName) {
         return loggerName;
     }
 
-    private static boolean initPending = true;
-    private static XRLogger loggerImpl;
+    private static volatile boolean initPending = true;
+    private static volatile XRLogger loggerImpl;
 
     private static volatile Boolean loggingEnabled;
 

diff --git a/openhtmltopdf-core/src/test/java/com/openhtmltopdf/util/XRLogTest.java b/openhtmltopdf-core/src/test/java/com/openhtmltopdf/util/XRLogTest.java
@@ -3,6 +3,8 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.logging.Level;
 
 public class XRLogTest {
@@ -14,5 +16,31 @@ public void testDisableLogBeforeFirstLog() {
 		XRLog.log(Level.INFO, LogMessageId.LogMessageId0Param.LOAD_UNABLE_TO_DISABLE_XML_EXTERNAL_ENTITIES);
 		Assert.assertFalse(XRLog.isLoggingEnabled());
 	}
+
+	/**
+	 * See issue https://github.com/danfickle/openhtmltopdf/issues/646
+	 */
+	@Test
+	public void testConcurrentInitLog() throws InterruptedException {
+		int p = 20;
+		CountDownLatch latch = new CountDownLatch(p);
+		CountDownLatch end = new CountDownLatch(p);
+		AtomicInteger counter = new AtomicInteger();
+		for (int i = 0; i < p;i++) {
+			new Thread(() -> {
+				latch.countDown();
+				try {
+					latch.await();
+					XRLog.log(Level.SEVERE, LogMessageId.LogMessageId0Param.CASCADE_IS_ABSOLUTE_CSS_UNKNOWN_GIVEN);
+					counter.incrementAndGet();
+					end.countDown();
+				} catch (Throwable e) {
+					end.countDown();
+				}
+			}).start();
+		}
+		end.await();
+		Assert.assertEquals(p, counter.get()); //we expect 0 NPE -> counter = 20
+	}
 
 }