Merge pull request #624 from rootvector2/csvformat-print-writelock

synchronize private CSVFormat print overload on writeLock

Author: garydgregory

src/main/java/org/apache/commons/csv/CSVFormat.java

@@ -2264,21 +2264,23 @@ public void print(final Object value, final Appendable out, final boolean newRec
         }
     }
 
-    private synchronized void print(final Object object, final CharSequence value, final Appendable out, final boolean newRecord) throws IOException {
-        final int offset = 0;
-        final int len = value.length();
-        if (!newRecord) {
-            out.append(getDelimiterString());
-        }
-        if (object == null) {
-            out.append(value);
-        } else if (isQuoteCharacterSet()) {
-            // The original object is needed so can check for Number
-            printWithQuotes(object, value, out, newRecord);
-        } else if (isEscapeCharacterSet()) {
-            printWithEscapes(value, out);
-        } else {
-            out.append(value, offset, len);
+    private void print(final Object object, final CharSequence value, final Appendable out, final boolean newRecord) throws IOException {
+        synchronized (writeLock) {
+            final int offset = 0;
+            final int len = value.length();
+            if (!newRecord) {
+                out.append(getDelimiterString());
+            }
+            if (object == null) {
+                out.append(value);
+            } else if (isQuoteCharacterSet()) {
+                // The original object is needed so can check for Number
+                printWithQuotes(object, value, out, newRecord);
+            } else if (isEscapeCharacterSet()) {
+                printWithEscapes(value, out);
+            } else {
+                out.append(value, offset, len);
+            }
         }
     }
 

src/test/java/org/apache/commons/csv/CSVFormatTest.java

@@ -31,6 +31,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotSame;
 import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTimeoutPreemptively;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -45,8 +46,11 @@
 import java.lang.reflect.Modifier;
 import java.sql.ResultSet;
 import java.sql.SQLException;
+import java.time.Duration;
 import java.util.Arrays;
 import java.util.Objects;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
 
 import org.apache.commons.csv.CSVFormat.Builder;
 import org.junit.jupiter.api.Test;
@@ -921,6 +925,37 @@ void testPrintRecordEmpty() throws IOException {
         assertEquals(format.getRecordSeparator(), out.toString());
     }
 
+    @Test
+    void testPrintRecordNotBlockedByInstanceMonitor() throws Exception {
+        // A CSVFormat is documented as thread-safe. Internal write locking uses a private lock, so external code
+        // holding the format instance monitor must not be able to block a print call.
+        final CSVFormat format = CSVFormat.RFC4180;
+        final CountDownLatch holdingMonitor = new CountDownLatch(1);
+        final CountDownLatch releaseMonitor = new CountDownLatch(1);
+        final Thread holder = new Thread(() -> {
+            synchronized (format) {
+                holdingMonitor.countDown();
+                try {
+                    releaseMonitor.await();
+                } catch (final InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                }
+            }
+        });
+        holder.start();
+        try {
+            assertTrue(holdingMonitor.await(5, TimeUnit.SECONDS));
+            assertTimeoutPreemptively(Duration.ofSeconds(5), () -> {
+                final Appendable out = new StringBuilder();
+                format.printRecord(out, "a", "b", "c");
+                assertEquals("a,b,c" + format.getRecordSeparator(), out.toString());
+            });
+        } finally {
+            releaseMonitor.countDown();
+            holder.join();
+        }
+    }
+
     @Test
     void testPrintWithEscapesEndWithCRLF() throws IOException {
         final Reader in = new StringReader("x,y,x\r\na,?b,c\r\n");