Fix StringBuilder cache corruption on recursive access

log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.internal;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.Queue;
+import java.util.Random;
+import java.util.concurrent.ArrayBlockingQueue;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+import org.junit.jupiter.params.provider.ValueSource;
+
+public class ArrayQueueTest {
+
+    @ParameterizedTest
+    @ValueSource(ints = {-1, 0})
+    void invalid_capacity_should_not_be_allowed(final int invalidCapacity) {
+        assertThatThrownBy(() -> new ArrayQueue<>(invalidCapacity))
+                .isInstanceOf(IllegalArgumentException.class)
+                .hasMessage("invalid capacity: " + invalidCapacity);
+    }
+
+    @Test
+    void should_work_with_capacity_1() {
+
+        // Verify initials
+        final Queue<String> queue = new ArrayQueue<>(1);
+        assertThat(queue.size()).isEqualTo(0);
+        assertThat(queue.peek()).isNull();
+        assertThat(queue.poll()).isNull();
+        assertThat(queue).isEmpty();
+
+        // Verify enqueue & deque
+        assertThat(queue.offer("foo")).isTrue();
+        assertThat(queue.offer("bar")).isFalse();
+        assertThat(queue.size()).isEqualTo(1);
+        assertThat(queue).containsOnly("foo");
+        assertThat(queue.peek()).isEqualTo("foo");
+        assertThat(queue.poll()).isEqualTo("foo");
+
+        // Verify final state
+        assertThat(queue.size()).isEqualTo(0);
+        assertThat(queue.peek()).isNull();
+        assertThat(queue.poll()).isNull();
+        assertThat(queue).isEmpty();
+    }
+
+    @ParameterizedTest
+    @CsvSource({
+        "1,0.3", "1,0.5", "1,0.8", "2,0.3", "2,0.5", "2,0.8", "3,0.3", "3,0.5", "3,0.8", "4,0.3", "4,0.5", "4,0.8"
+    })
+    void ops_should_match_with_std_lib(final int capacity, final double pollRatio) {
+
+        // Set the stage
+        final Random random = new Random(0);
+        final int opCount = random.nextInt(100);
+        final Queue<String> queueRef = new ArrayBlockingQueue<>(capacity);
+        final Queue<String> queueTarget = new ArrayQueue<>(capacity);
+
+        for (int opIndex = 0; opIndex < opCount; opIndex++) {
+
+            // Verify entry
+            assertThat(queueTarget.size()).isEqualTo(queueRef.size());
+            assertThat(queueTarget.peek()).isEqualTo(queueRef.peek());
+            assertThat(queueTarget).containsExactlyElementsOf(queueRef);
+
+            // Is this a `poll()`?
+            if (pollRatio >= random.nextDouble()) {
+                assertThat(queueTarget.poll()).isEqualTo(queueRef.poll());
+            }
+
+            // Then this is an `offer()`
+            else {
+                final String item = "op@" + opIndex;
+                assertThat(queueTarget.offer(item)).isEqualTo(queueRef.offer(item));
+            }
+
+            // Verify exit
+            assertThat(queueTarget.size()).isEqualTo(queueRef.size());
+            assertThat(queueTarget.peek()).isEqualTo(queueRef.peek());
+            assertThat(queueTarget).containsExactlyElementsOf(queueRef);
+        }
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingTestBase.java

@@ -0,0 +1,53 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.message;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.apache.logging.log4j.util.Constants;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Tests {@link ParameterizedMessage#getFormattedMessage()} when formatted arguments causes another {@code ParameterizedMessage#getFormattedMessage()} (i.e., recursive) invocation.
+ */
+abstract class ParameterizedMessageRecursiveFormattingTestBase {
+
+    private final boolean threadLocalsEnabled;
+
+    ParameterizedMessageRecursiveFormattingTestBase(boolean threadLocalsEnabled) {
+        this.threadLocalsEnabled = threadLocalsEnabled;
+    }
+
+    @Test
+    void thread_locals_toggle_should_match() {
+        assertThat(Constants.ENABLE_THREADLOCALS).isEqualTo(threadLocalsEnabled);
+    }
+
+    @Test
+    void recursion_should_not_corrupt_formatting() {
+        final Object argInvokingParameterizedMessageFormatting = new Object() {
+            @Override
+            public String toString() {
+                return new ParameterizedMessage("bar {}", "baz").getFormattedMessage();
+            }
+        };
+        final ParameterizedMessage message =
+                new ParameterizedMessage("foo {}", argInvokingParameterizedMessageFormatting);
+        final String actualText = message.getFormattedMessage();
+        assertThat(actualText).isEqualTo("foo bar baz");
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithThreadLocalsTest.java

@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.message;
+
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+/**
+ * {@link ParameterizedMessageRecursiveFormattingTestBase} subclass with thread locals disabled, i.e, with {@link StringBuilder} recycling.
+ */
+@SetSystemProperty(key = "log4j2.enable.threadlocals", value = "true")
+class ParameterizedMessageRecursiveFormattingWithThreadLocalsTest
+        extends ParameterizedMessageRecursiveFormattingTestBase {
+
+    ParameterizedMessageRecursiveFormattingWithThreadLocalsTest() {
+        super(true);
+    }
+}

log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest.java

@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.message;
+
+import org.junitpioneer.jupiter.SetSystemProperty;
+
+/**
+ * {@link ParameterizedMessageRecursiveFormattingTestBase} subclass with thread locals disabled, i.e, no {@link StringBuilder} recycling.
+ */
+@SetSystemProperty(key = "log4j2.enable.threadlocals", value = "false")
+class ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest
+        extends ParameterizedMessageRecursiveFormattingTestBase {
+
+    ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest() {
+        super(false);
+    }
+}

log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java

@@ -0,0 +1,101 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.internal;
+
+import java.util.AbstractQueue;
+import java.util.Iterator;
+import java.util.stream.IntStream;
+import javax.annotation.concurrent.NotThreadSafe;
+import org.apache.logging.log4j.util.InternalApi;
+
+/**
+ * An array-backed, fixed-length, not-thread-safe {@link java.util.Queue} implementation.
+ *
+ * @param <E> the element type
+ * @since 2.23.0
+ */
+@InternalApi
+@NotThreadSafe
+final class ArrayQueue<E> extends AbstractQueue<E> {
+
+    private final E[] buffer;
+
+    private int head;
+
+    private int tail;
+
+    private int size;
+
+    @SuppressWarnings("unchecked")
+    ArrayQueue(final int capacity) {
+        if (capacity < 1) {
+            throw new IllegalArgumentException("invalid capacity: " + capacity);
+        }
+        buffer = (E[]) new Object[capacity];
+        head = 0;
+        tail = -1;
+        size = 0;
+    }
+
+    @Override
+    public Iterator<E> iterator() {
+        int[] i = {head};
+        return IntStream.range(0, size)
+                .mapToObj(ignored -> {
+                    final E item = buffer[i[0]];
+                    i[0] = (i[0] + 1) % buffer.length;
+                    return item;
+                })
+                .iterator();
+    }
+
+    @Override
+    public boolean offer(final E item) {
+        if (size == buffer.length) {
+            return false;
+        }
+        tail = (tail + 1) % buffer.length;
+        buffer[tail] = item;
+        size++;
+        return true;
+    }
+
+    @Override
+    public E poll() {
+        if (isEmpty()) {
+            return null;
+        }
+        final E item = buffer[head];
+        buffer[head] = null; // Clear refs for GC
+        head = (head + 1) % buffer.length;
+        size--;
+        return item;
+    }
+
+    @Override
+    public E peek() {
+        if (isEmpty()) {
+            return null;
+        }
+        return buffer[head];
+    }
+
+    @Override
+    public int size() {
+        return size;
+    }
+}

log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java

@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to you under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.logging.log4j.internal;
+
+/**
+ * A {@link StringBuilderRecycler} that <b>always</b> allocates a new instance.
+ *
+ * @since 2.23.0
+ */
+final class DummyStringBuilderRecycler implements StringBuilderRecycler {
+
+    DummyStringBuilderRecycler() {}
+
+    @Override
+    public StringBuilder acquire() {
+        return new StringBuilder();
+    }
+
+    @Override
+    public void release(final StringBuilder stringBuilder) {}
+}