Merge pull request #1159 from the-thing/string-builder-trim

Trimming oversized `Message StringBuilder` to page size

Author: chrjohn

quickfixj-base/src/main/java/quickfix/Message.java

@@ -150,17 +150,14 @@ private Object cloneTo(Message message) {
     }
 
     private static final class Context {
+
+        private static final int MAX_MESSAGE_BUFFER_SIZE = 4096;
         private final BodyLength bodyLength = new BodyLength(100);
         private final CheckSum checkSum = new CheckSum("000");
         private final StringBuilder stringBuilder = new StringBuilder(1024);
     }
 
-    private static final ThreadLocal<Context> stringContexts = new ThreadLocal<Context>() {
-        @Override
-        protected Context initialValue() {
-            return new Context();
-        }
-    };
+    private static final ThreadLocal<Context> STRING_CONTEXTS = ThreadLocal.withInitial(Context::new);
 
     /**
      * Do not call this method concurrently while modifying the contents of the message.
@@ -173,7 +170,7 @@ protected Context initialValue() {
      */
     @Override
     public String toString() {
-        Context context = stringContexts.get();
+        Context context = STRING_CONTEXTS.get();
         if (CharsetSupport.isStringEquivalent()) { // length & checksum can easily be calculated after message is built
             header.setField(context.bodyLength);
             trailer.setField(context.checkSum);
@@ -192,6 +189,11 @@ public String toString() {
             }
             return stringBuilder.toString();
         } finally {
+            if (stringBuilder.length() > Context.MAX_MESSAGE_BUFFER_SIZE) {
+                stringBuilder.setLength(Context.MAX_MESSAGE_BUFFER_SIZE);
+                stringBuilder.trimToSize();
+            }
+
             stringBuilder.setLength(0);
         }
     }
@@ -1018,4 +1020,7 @@ void setGarbled(boolean isGarbled) {
         this.isGarbled = isGarbled;
     }
 
+    StringBuilder getStringBuilder() {
+        return STRING_CONTEXTS.get().stringBuilder;
+    }
 }

quickfixj-base/src/test/java/quickfix/MessageTest.java

@@ -13,6 +13,8 @@
 import java.time.ZoneOffset;
 import java.util.Calendar;
 import java.util.TimeZone;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Executors;
 
 import org.junit.Rule;
 import org.junit.Test;
@@ -746,4 +748,45 @@ public void testHeaderDataField() throws Exception {
                 DataDictionaryTest.getDictionary());
         assertEquals("ABCD", m.getHeader().getString(SecureData.FIELD));
     }
+
+    @Test
+    public void shouldTrimStringBuilder() {
+        // this test must run in a dedicated thread to avoid interference with other test cases (thread local)
+        CompletableFuture<?> future = CompletableFuture.runAsync(() -> {
+            Message message = new Message();
+
+            message.setString(131, "123456");
+            String str = message.toString();
+
+            assertEquals(23, str.length());
+            assertEquals(0, message.getStringBuilder().length());
+            assertEquals(1024, message.getStringBuilder().capacity());
+
+            message.setString(131, createLongString());
+            str = message.toString();
+
+            assertEquals(10020, str.length());
+            assertEquals(0, message.getStringBuilder().length());
+            assertEquals(4096, message.getStringBuilder().capacity());
+
+            message.setString(131, "123456");
+            str = message.toString();
+
+            assertEquals(23, str.length());
+            assertEquals(0, message.getStringBuilder().length());
+            assertEquals(4096, message.getStringBuilder().capacity());
+        }, Executors.newSingleThreadExecutor());
+
+        future.join();
+    }
+
+    private static String createLongString() {
+        StringBuilder builder = new StringBuilder(10_000);
+
+        for (int i = 0; i < 10_000; i++) {
+            builder.append('a');
+        }
+
+        return builder.toString();
+    }
 }