From a258a589234d3fe11da7f0f7656462ad714ed1ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 9 Feb 2024 10:48:06 +0100 Subject: [PATCH 1/5] Fix `StringBuilder` cache corruption on recursive access --- .../log4j/internal/ArrayQueueTest.java | 101 ++++++++++++++++++ ...zedMessageRecursiveFormattingTestBase.java | 53 +++++++++ ...cursiveFormattingWithThreadLocalsTest.java | 31 ++++++ ...siveFormattingWithoutThreadLocalsTest.java | 31 ++++++ .../logging/log4j/internal/ArrayQueue.java | 101 ++++++++++++++++++ .../internal/DummyStringBuilderRecycler.java | 35 ++++++ ...ueingThreadLocalStringBuilderRecycler.java | 55 ++++++++++ .../log4j/internal/StringBuilderRecycler.java | 61 +++++++++++ .../ThreadLocalStringBuilderRecycler.java | 54 ++++++++++ .../log4j/message/ParameterizedMessage.java | 50 ++++----- .../apache/logging/log4j/util/Strings.java | 11 +- .../.2.x.x/fix_api_recursive_formatting.xml | 9 ++ 12 files changed, 562 insertions(+), 30 deletions(-) create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingTestBase.java create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithThreadLocalsTest.java create mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java create mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java create mode 100644 src/changelog/.2.x.x/fix_api_recursive_formatting.xml diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java new file mode 100644 index 00000000000..e063e7f81cf --- /dev/null +++ b/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 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 queueRef = new ArrayBlockingQueue<>(capacity); + final Queue 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); + } + } +} diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingTestBase.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingTestBase.java new file mode 100644 index 00000000000..072696029a7 --- /dev/null +++ b/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"); + } +} diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithThreadLocalsTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithThreadLocalsTest.java new file mode 100644 index 00000000000..6496a5f12f2 --- /dev/null +++ b/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); + } +} diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageRecursiveFormattingWithoutThreadLocalsTest.java new file mode 100644 index 00000000000..456d271d872 --- /dev/null +++ b/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); + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java new file mode 100644 index 00000000000..6777d569300 --- /dev/null +++ b/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 the element type + * @since 2.23.0 + */ +@InternalApi +@NotThreadSafe +final class ArrayQueue extends AbstractQueue { + + 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 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; + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java new file mode 100644 index 00000000000..9deecccce54 --- /dev/null +++ b/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 always 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) {} +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java new file mode 100644 index 00000000000..d867a8df280 --- /dev/null +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java @@ -0,0 +1,55 @@ +/* + * 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 edu.umd.cs.findbugs.annotations.Nullable; +import java.util.Queue; +import org.apache.logging.log4j.util.StringBuilders; + +/** + * A {@link StringBuilderRecycler} that keeps a {@link ThreadLocal} queue to recycle instances. + *

+ * When the queue capacity is exceeded, {@link #acquire()} starts allocating new instances. + * The queue provides convenience when recursive {@link #acquire()} calls are expected. + * If not, use {@link ThreadLocalStringBuilderRecycler} instead to avoid the cost of maintaining a queue. + *

+ * + * @since 2.23.0 + */ +final class QueueingThreadLocalStringBuilderRecycler implements StringBuilderRecycler { + + private final ThreadLocal> queueHolder; + + QueueingThreadLocalStringBuilderRecycler(final int threadLocalQueueCapacity) { + this.queueHolder = ThreadLocal.withInitial(() -> new ArrayQueue<>(threadLocalQueueCapacity)); + } + + @Override + public StringBuilder acquire() { + Queue queue = queueHolder.get(); + @Nullable final StringBuilder stringBuilder = queue.poll(); + return stringBuilder != null ? stringBuilder : new StringBuilder(); + } + + @Override + public void release(final StringBuilder stringBuilder) { + StringBuilders.trimToMaxSize(stringBuilder, MAX_STRING_BUILDER_CAPACITY); + stringBuilder.setLength(0); + final Queue queue = queueHolder.get(); + queue.offer(stringBuilder); + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java new file mode 100644 index 00000000000..3ff96c11525 --- /dev/null +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java @@ -0,0 +1,61 @@ +/* + * 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 org.apache.logging.log4j.util.Constants; +import org.apache.logging.log4j.util.InternalApi; + +/** + * An internal tool to recycle {@link StringBuilder} instances to implement garbage-free formatting. + *

Internal usage only!

+ *

+ * This interface is only intended to be used for internals of the Log4j API. + * Log4j API can break this contract at any time to adapt to its needs. + * {@link StringBuilderRecycler} should not be used by any other code except the Log4j API! + *

+ * + * @since 2.23.0 + */ +@InternalApi +public interface StringBuilderRecycler { + + int MAX_STRING_BUILDER_CAPACITY = Constants.MAX_REUSABLE_MESSAGE_SIZE; + + StringBuilder acquire(); + + void release(final StringBuilder stringBuilder); + + /** + * Creates a {@link StringBuilderRecycler} using the current environment, e.g., {@link Constants}, {@link org.apache.logging.log4j.util.PropertiesUtil}, etc. + * + * @param recursionDepth This value indicates the maximum recursion depth to be supported before falling back to allocating new instances. + * For instance, {@link QueueingThreadLocalStringBuilderRecycler} uses this to set the capacity of the instances to be pooled in a {@link ThreadLocal}. + * @return a new {@link StringBuilderRecycler} instance + */ + static StringBuilderRecycler ofEnvironment(final int recursionDepth) { + if (recursionDepth < 0) { + throw new IllegalArgumentException("was expecting a positive `recursionDepth`, found: " + recursionDepth); + } + if (!Constants.ENABLE_THREADLOCALS) { + return new DummyStringBuilderRecycler(); + } else if (recursionDepth == 0) { + return new ThreadLocalStringBuilderRecycler(); + } else { + return new QueueingThreadLocalStringBuilderRecycler(recursionDepth); + } + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java new file mode 100644 index 00000000000..6638fb6e6ae --- /dev/null +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java @@ -0,0 +1,54 @@ +/* + * 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 edu.umd.cs.findbugs.annotations.Nullable; +import org.apache.logging.log4j.util.StringBuilders; + +/** + * A {@link StringBuilderRecycler} that stores a single {@link StringBuilder} in a {@link ThreadLocal}. + *

+ * This implementation causes new instance creation when {@link #acquire()} is called recursively. + * If your use case needs recursion support, prefer {@link QueueingThreadLocalStringBuilderRecycler} instead. + *

+ * + * @since 2.23.0 + */ +final class ThreadLocalStringBuilderRecycler implements StringBuilderRecycler { + + private final ThreadLocal stringBuilderHolder = ThreadLocal.withInitial(StringBuilder::new); + + ThreadLocalStringBuilderRecycler() {} + + @Override + public StringBuilder acquire() { + @Nullable StringBuilder stringBuilder = stringBuilderHolder.get(); + if (stringBuilder == null) { + return new StringBuilder(); + } + // noinspection ThreadLocalSetWithNull + stringBuilderHolder.set(null); + return stringBuilder; + } + + @Override + public void release(final StringBuilder stringBuilder) { + StringBuilders.trimToMaxSize(stringBuilder, MAX_STRING_BUILDER_CAPACITY); + stringBuilder.setLength(0); + stringBuilderHolder.set(stringBuilder); + } +} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index db325327b71..30677a85658 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -25,10 +25,9 @@ import java.io.Serializable; import java.util.Arrays; import java.util.Objects; +import org.apache.logging.log4j.internal.StringBuilderRecycler; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; -import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.StringBuilderFormattable; -import org.apache.logging.log4j.util.StringBuilders; import org.apache.logging.log4j.util.internal.SerializationUtil; /** @@ -44,10 +43,6 @@ * {bar * {buzz} * - *

- * This class was originally written for Lilith by Jörn Huxhorn and licensed under the LGPL. - * It has been relicensed here with his permission providing that this attribution remain. - *

*/ public class ParameterizedMessage implements Message, StringBuilderFormattable { @@ -83,9 +78,15 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable { private static final long serialVersionUID = -665975803997290697L; - private static final ThreadLocal STRING_BUILDER_HOLDER = Constants.ENABLE_THREADLOCALS - ? ThreadLocal.withInitial(() -> new StringBuilder(Constants.MAX_REUSABLE_MESSAGE_SIZE)) - : null; + private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.ofEnvironment( + // This value indicates the maximum recursion depth supported before the recycler starts creating new + // instances. + // Consider a `ParameterizedMessage` containing an argument whose `toString()` causes another + // `ParameterizedMessage` formatting. + // This value indicates the depth we support garbage-free formatting in such nested formatting situations. + // When this depth is exceeded, code still works, but starts generating garbage due to new `StringBuilder` + // allocations. + 3); private final String pattern; @@ -242,16 +243,13 @@ public Throwable getThrowable() { @Override public String getFormattedMessage() { if (formattedMessage == null) { - if (STRING_BUILDER_HOLDER != null) { - final StringBuilder buffer = STRING_BUILDER_HOLDER.get(); + final StringBuilder buffer = STRING_BUILDER_RECYCLER.acquire(); + try { buffer.setLength(0); formatTo(buffer); formattedMessage = buffer.toString(); - StringBuilders.trimToMaxSize(buffer, Constants.MAX_REUSABLE_MESSAGE_SIZE); - } else { - final StringBuilder buffer = new StringBuilder(); - formatTo(buffer); - formattedMessage = buffer.toString(); + } finally { + STRING_BUILDER_RECYCLER.release(buffer); } } return formattedMessage; @@ -355,26 +353,28 @@ public static String identityToString(final Object obj) { @Override public String toString() { - return "ParameterizedMessage[messagePattern=" + pattern + ", stringArgs=" + Arrays.toString(args) - + ", throwable=" + throwable + ']'; + // Avoid formatting arguments! + // It can cause recursion, which can become pretty unpleasant while troubleshooting. + return "ParameterizedMessage[messagePattern=" + pattern + ", argCount=" + args.length + ", throwableProvided=" + + (throwable != null) + ']'; } private void writeObject(final ObjectOutputStream out) throws IOException { out.defaultWriteObject(); out.writeInt(args.length); - for (int i = 0; i < args.length; i++) { - SerializationUtil.writeWrappedObject( - args[i] instanceof Serializable ? (Serializable) args[i] : String.valueOf(args[i]), out); + for (final Object arg : args) { + final Serializable serializableArg = arg instanceof Serializable ? (Serializable) arg : String.valueOf(arg); + SerializationUtil.writeWrappedObject(serializableArg, out); } } private void readObject(final ObjectInputStream in) throws IOException, ClassNotFoundException { SerializationUtil.assertFiltered(in); in.defaultReadObject(); - final int length = in.readInt(); - args = new Object[length]; - for (int i = 0; i < args.length; i++) { - args[i] = SerializationUtil.readWrappedObject(in); + final int argCount = in.readInt(); + args = new Object[argCount]; + for (int argIndex = 0; argIndex < args.length; argIndex++) { + args[argIndex] = SerializationUtil.readWrappedObject(in); } } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java index a1a5bb1aab8..512784d3c87 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java @@ -19,6 +19,7 @@ import java.util.Iterator; import java.util.Locale; import java.util.Objects; +import org.apache.logging.log4j.internal.StringBuilderRecycler; /** * Consider this class private. @@ -28,7 +29,7 @@ @InternalApi public final class Strings { - private static final ThreadLocal tempStr = ThreadLocal.withInitial(StringBuilder::new); + private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.ofEnvironment(0); /** * The empty string. @@ -318,11 +319,11 @@ public static String concat(final String str1, final String str2) { } else if (isEmpty(str2)) { return str1; } - final StringBuilder sb = tempStr.get(); + final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); try { return sb.append(str1).append(str2).toString(); } finally { - sb.setLength(0); + STRING_BUILDER_RECYCLER.release(sb); } } @@ -338,14 +339,14 @@ public static String repeat(final String str, final int count) { if (count < 0) { throw new IllegalArgumentException("count"); } - final StringBuilder sb = tempStr.get(); + final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); try { for (int index = 0; index < count; index++) { sb.append(str); } return sb.toString(); } finally { - sb.setLength(0); + STRING_BUILDER_RECYCLER.release(sb); } } } diff --git a/src/changelog/.2.x.x/fix_api_recursive_formatting.xml b/src/changelog/.2.x.x/fix_api_recursive_formatting.xml new file mode 100644 index 00000000000..558152b60ce --- /dev/null +++ b/src/changelog/.2.x.x/fix_api_recursive_formatting.xml @@ -0,0 +1,9 @@ + + + + Fix `StringBuilder` cache corruption on recursive access + + From 2ca7a5ae76bd2641c7ea839eca07b48f1f2599b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 9 Feb 2024 11:32:37 +0100 Subject: [PATCH 2/5] Use recycler in `ReusableParameterizedMessage` --- ...ueingThreadLocalStringBuilderRecycler.java | 7 ++- .../log4j/internal/StringBuilderRecycler.java | 20 ++++++--- .../ThreadLocalStringBuilderRecycler.java | 8 +++- .../log4j/message/ParameterizedMessage.java | 19 ++++---- .../message/ReusableParameterizedMessage.java | 45 +++++++++---------- .../apache/logging/log4j/util/Strings.java | 5 ++- 6 files changed, 60 insertions(+), 44 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java index d867a8df280..3d42889a259 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java @@ -32,9 +32,12 @@ */ final class QueueingThreadLocalStringBuilderRecycler implements StringBuilderRecycler { + private final int maxLength; + private final ThreadLocal> queueHolder; - QueueingThreadLocalStringBuilderRecycler(final int threadLocalQueueCapacity) { + QueueingThreadLocalStringBuilderRecycler(final int maxLength, final int threadLocalQueueCapacity) { + this.maxLength = maxLength; this.queueHolder = ThreadLocal.withInitial(() -> new ArrayQueue<>(threadLocalQueueCapacity)); } @@ -47,7 +50,7 @@ public StringBuilder acquire() { @Override public void release(final StringBuilder stringBuilder) { - StringBuilders.trimToMaxSize(stringBuilder, MAX_STRING_BUILDER_CAPACITY); + StringBuilders.trimToMaxSize(stringBuilder, maxLength); stringBuilder.setLength(0); final Queue queue = queueHolder.get(); queue.offer(stringBuilder); diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java index 3ff96c11525..7abc3b44c2f 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java @@ -33,8 +33,6 @@ @InternalApi public interface StringBuilderRecycler { - int MAX_STRING_BUILDER_CAPACITY = Constants.MAX_REUSABLE_MESSAGE_SIZE; - StringBuilder acquire(); void release(final StringBuilder stringBuilder); @@ -42,20 +40,28 @@ public interface StringBuilderRecycler { /** * Creates a {@link StringBuilderRecycler} using the current environment, e.g., {@link Constants}, {@link org.apache.logging.log4j.util.PropertiesUtil}, etc. * + * @param maxLength The length recycled {@link StringBuilder}s will be trimmed to * @param recursionDepth This value indicates the maximum recursion depth to be supported before falling back to allocating new instances. * For instance, {@link QueueingThreadLocalStringBuilderRecycler} uses this to set the capacity of the instances to be pooled in a {@link ThreadLocal}. + * @param threadLocalsEnabled Indicates if the {@link ThreadLocal} usage is allowed * @return a new {@link StringBuilderRecycler} instance */ - static StringBuilderRecycler ofEnvironment(final int recursionDepth) { + static StringBuilderRecycler of( + final int maxLength, + final int recursionDepth, + final boolean threadLocalsEnabled) { + if (maxLength < 1) { + throw new IllegalArgumentException("was expecting `maxLength > 0`, found: " + maxLength); + } if (recursionDepth < 0) { - throw new IllegalArgumentException("was expecting a positive `recursionDepth`, found: " + recursionDepth); + throw new IllegalArgumentException("was expecting `recursionDepth >= 0`, found: " + recursionDepth); } - if (!Constants.ENABLE_THREADLOCALS) { + if (!threadLocalsEnabled) { return new DummyStringBuilderRecycler(); } else if (recursionDepth == 0) { - return new ThreadLocalStringBuilderRecycler(); + return new ThreadLocalStringBuilderRecycler(maxLength); } else { - return new QueueingThreadLocalStringBuilderRecycler(recursionDepth); + return new QueueingThreadLocalStringBuilderRecycler(maxLength, recursionDepth); } } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java index 6638fb6e6ae..81a71c842c4 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java @@ -30,9 +30,13 @@ */ final class ThreadLocalStringBuilderRecycler implements StringBuilderRecycler { + private final int maxLength; + private final ThreadLocal stringBuilderHolder = ThreadLocal.withInitial(StringBuilder::new); - ThreadLocalStringBuilderRecycler() {} + ThreadLocalStringBuilderRecycler(final int maxLength) { + this.maxLength = maxLength; + } @Override public StringBuilder acquire() { @@ -47,7 +51,7 @@ public StringBuilder acquire() { @Override public void release(final StringBuilder stringBuilder) { - StringBuilders.trimToMaxSize(stringBuilder, MAX_STRING_BUILDER_CAPACITY); + StringBuilders.trimToMaxSize(stringBuilder, maxLength); stringBuilder.setLength(0); stringBuilderHolder.set(stringBuilder); } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index 30677a85658..eedfd14acb5 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -27,6 +27,7 @@ import java.util.Objects; import org.apache.logging.log4j.internal.StringBuilderRecycler; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; +import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.StringBuilderFormattable; import org.apache.logging.log4j.util.internal.SerializationUtil; @@ -78,15 +79,15 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable { private static final long serialVersionUID = -665975803997290697L; - private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.ofEnvironment( - // This value indicates the maximum recursion depth supported before the recycler starts creating new - // instances. - // Consider a `ParameterizedMessage` containing an argument whose `toString()` causes another - // `ParameterizedMessage` formatting. - // This value indicates the depth we support garbage-free formatting in such nested formatting situations. - // When this depth is exceeded, code still works, but starts generating garbage due to new `StringBuilder` - // allocations. - 3); + private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( + Constants.MAX_REUSABLE_MESSAGE_SIZE, + // This value indicates the maximum recursion depth before the recycler starts creating new instances. + // Consider a `ParameterizedMessage` containing an argument such that its `toString()` causes another (i.e., + // recursive) `ParameterizedMessage` formatting. This value indicates the depth we support garbage-free + // formatting in such nested formatting situations. When this depth is exceeded, code still works, but + // starts generating garbage due to new `StringBuilder` allocations. + 3, + Constants.ENABLE_THREADLOCALS); private final String pattern; diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java index b9b73a6d007..a3b8c9cb807 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java @@ -17,10 +17,11 @@ package org.apache.logging.log4j.message; import java.util.Arrays; + +import org.apache.logging.log4j.internal.StringBuilderRecycler; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.PerformanceSensitive; -import org.apache.logging.log4j.util.StringBuilders; /** * Reusable parameterized message. This message is mutable and is not safe to be accessed or modified by multiple @@ -32,10 +33,18 @@ @PerformanceSensitive("allocation") public class ReusableParameterizedMessage implements ReusableMessage, ParameterVisitable, Clearable { - private static final int MIN_BUILDER_SIZE = 512; private static final int MAX_PARAMS = 10; private static final long serialVersionUID = 7800075879295123856L; - private transient ThreadLocal buffer; // non-static: LOG4J2-1583 + + private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( + Constants.MAX_REUSABLE_MESSAGE_SIZE, + // This value indicates the maximum recursion depth before the recycler starts creating new instances. + // Consider a `ParameterizedMessage` containing an argument such that its `toString()` causes another (i.e., + // recursive) `ParameterizedMessage` formatting. This value indicates the depth we support garbage-free + // formatting in such nested formatting situations. When this depth is exceeded, code still works, but + // starts generating garbage due to new `StringBuilder` allocations. + 3, + Constants.ENABLE_THREADLOCALS); private String messagePattern; private final MessagePatternAnalysis patternAnalysis = new MessagePatternAnalysis(); @@ -336,25 +345,13 @@ public Throwable getThrowable() { */ @Override public String getFormattedMessage() { - final StringBuilder sb = getBuffer(); - formatTo(sb); - final String result = sb.toString(); - StringBuilders.trimToMaxSize(sb, Constants.MAX_REUSABLE_MESSAGE_SIZE); - return result; - } - - private StringBuilder getBuffer() { - if (buffer == null) { - buffer = new ThreadLocal<>(); - } - StringBuilder result = buffer.get(); - if (result == null) { - final int currentPatternLength = messagePattern == null ? 0 : messagePattern.length(); - result = new StringBuilder(Math.max(MIN_BUILDER_SIZE, currentPatternLength * 2)); - buffer.set(result); + final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); + try { + formatTo(sb); + return sb.toString(); + } finally { + STRING_BUILDER_RECYCLER.release(sb); } - result.setLength(0); - return result; } @Override @@ -374,8 +371,10 @@ ReusableParameterizedMessage reserve() { @Override public String toString() { - return "ReusableParameterizedMessage[messagePattern=" + getFormat() + ", stringArgs=" - + Arrays.toString(getParameters()) + ", throwable=" + getThrowable() + ']'; + // Avoid formatting arguments! + // It can cause recursion, which can become pretty unpleasant while troubleshooting. + return "ReusableParameterizedMessage[messagePattern=" + getFormat() + ", argCount=" + getParameterCount() + + ", throwableProvided=" + (getThrowable() != null) + ']'; } @Override diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java index 512784d3c87..dcb0e4d4e0e 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java @@ -29,7 +29,10 @@ @InternalApi public final class Strings { - private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.ofEnvironment(0); + private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( + Constants.MAX_REUSABLE_MESSAGE_SIZE, + 0, + Constants.ENABLE_THREADLOCALS); /** * The empty string. From 2ddca4e21c9fc127064477633927562e051ee33a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 9 Feb 2024 20:42:47 +0100 Subject: [PATCH 3/5] Switch to a simpler implementation --- .../log4j/internal/ArrayQueueTest.java | 101 ------------------ .../logging/log4j/internal/ArrayQueue.java | 101 ------------------ .../internal/DummyStringBuilderRecycler.java | 35 ------ ...ueingThreadLocalStringBuilderRecycler.java | 58 ---------- .../log4j/internal/StringBuilderRecycler.java | 67 ------------ .../ThreadLocalStringBuilderRecycler.java | 58 ---------- .../log4j/message/ParameterizedMessage.java | 56 ++++++---- .../message/ReusableParameterizedMessage.java | 26 ++--- .../apache/logging/log4j/util/Strings.java | 22 ++-- 9 files changed, 57 insertions(+), 467 deletions(-) delete mode 100644 log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java delete mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java delete mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java delete mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java delete mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java delete mode 100644 log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java deleted file mode 100644 index e063e7f81cf..00000000000 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/internal/ArrayQueueTest.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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 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 queueRef = new ArrayBlockingQueue<>(capacity); - final Queue 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); - } - } -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java deleted file mode 100644 index 6777d569300..00000000000 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ArrayQueue.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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 the element type - * @since 2.23.0 - */ -@InternalApi -@NotThreadSafe -final class ArrayQueue extends AbstractQueue { - - 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 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; - } -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java deleted file mode 100644 index 9deecccce54..00000000000 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/DummyStringBuilderRecycler.java +++ /dev/null @@ -1,35 +0,0 @@ -/* - * 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 always 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) {} -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java deleted file mode 100644 index 3d42889a259..00000000000 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/QueueingThreadLocalStringBuilderRecycler.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * 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 edu.umd.cs.findbugs.annotations.Nullable; -import java.util.Queue; -import org.apache.logging.log4j.util.StringBuilders; - -/** - * A {@link StringBuilderRecycler} that keeps a {@link ThreadLocal} queue to recycle instances. - *

- * When the queue capacity is exceeded, {@link #acquire()} starts allocating new instances. - * The queue provides convenience when recursive {@link #acquire()} calls are expected. - * If not, use {@link ThreadLocalStringBuilderRecycler} instead to avoid the cost of maintaining a queue. - *

- * - * @since 2.23.0 - */ -final class QueueingThreadLocalStringBuilderRecycler implements StringBuilderRecycler { - - private final int maxLength; - - private final ThreadLocal> queueHolder; - - QueueingThreadLocalStringBuilderRecycler(final int maxLength, final int threadLocalQueueCapacity) { - this.maxLength = maxLength; - this.queueHolder = ThreadLocal.withInitial(() -> new ArrayQueue<>(threadLocalQueueCapacity)); - } - - @Override - public StringBuilder acquire() { - Queue queue = queueHolder.get(); - @Nullable final StringBuilder stringBuilder = queue.poll(); - return stringBuilder != null ? stringBuilder : new StringBuilder(); - } - - @Override - public void release(final StringBuilder stringBuilder) { - StringBuilders.trimToMaxSize(stringBuilder, maxLength); - stringBuilder.setLength(0); - final Queue queue = queueHolder.get(); - queue.offer(stringBuilder); - } -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java deleted file mode 100644 index 7abc3b44c2f..00000000000 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/StringBuilderRecycler.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * 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 org.apache.logging.log4j.util.Constants; -import org.apache.logging.log4j.util.InternalApi; - -/** - * An internal tool to recycle {@link StringBuilder} instances to implement garbage-free formatting. - *

Internal usage only!

- *

- * This interface is only intended to be used for internals of the Log4j API. - * Log4j API can break this contract at any time to adapt to its needs. - * {@link StringBuilderRecycler} should not be used by any other code except the Log4j API! - *

- * - * @since 2.23.0 - */ -@InternalApi -public interface StringBuilderRecycler { - - StringBuilder acquire(); - - void release(final StringBuilder stringBuilder); - - /** - * Creates a {@link StringBuilderRecycler} using the current environment, e.g., {@link Constants}, {@link org.apache.logging.log4j.util.PropertiesUtil}, etc. - * - * @param maxLength The length recycled {@link StringBuilder}s will be trimmed to - * @param recursionDepth This value indicates the maximum recursion depth to be supported before falling back to allocating new instances. - * For instance, {@link QueueingThreadLocalStringBuilderRecycler} uses this to set the capacity of the instances to be pooled in a {@link ThreadLocal}. - * @param threadLocalsEnabled Indicates if the {@link ThreadLocal} usage is allowed - * @return a new {@link StringBuilderRecycler} instance - */ - static StringBuilderRecycler of( - final int maxLength, - final int recursionDepth, - final boolean threadLocalsEnabled) { - if (maxLength < 1) { - throw new IllegalArgumentException("was expecting `maxLength > 0`, found: " + maxLength); - } - if (recursionDepth < 0) { - throw new IllegalArgumentException("was expecting `recursionDepth >= 0`, found: " + recursionDepth); - } - if (!threadLocalsEnabled) { - return new DummyStringBuilderRecycler(); - } else if (recursionDepth == 0) { - return new ThreadLocalStringBuilderRecycler(maxLength); - } else { - return new QueueingThreadLocalStringBuilderRecycler(maxLength, recursionDepth); - } - } -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java b/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java deleted file mode 100644 index 81a71c842c4..00000000000 --- a/log4j-api/src/main/java/org/apache/logging/log4j/internal/ThreadLocalStringBuilderRecycler.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * 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 edu.umd.cs.findbugs.annotations.Nullable; -import org.apache.logging.log4j.util.StringBuilders; - -/** - * A {@link StringBuilderRecycler} that stores a single {@link StringBuilder} in a {@link ThreadLocal}. - *

- * This implementation causes new instance creation when {@link #acquire()} is called recursively. - * If your use case needs recursion support, prefer {@link QueueingThreadLocalStringBuilderRecycler} instead. - *

- * - * @since 2.23.0 - */ -final class ThreadLocalStringBuilderRecycler implements StringBuilderRecycler { - - private final int maxLength; - - private final ThreadLocal stringBuilderHolder = ThreadLocal.withInitial(StringBuilder::new); - - ThreadLocalStringBuilderRecycler(final int maxLength) { - this.maxLength = maxLength; - } - - @Override - public StringBuilder acquire() { - @Nullable StringBuilder stringBuilder = stringBuilderHolder.get(); - if (stringBuilder == null) { - return new StringBuilder(); - } - // noinspection ThreadLocalSetWithNull - stringBuilderHolder.set(null); - return stringBuilder; - } - - @Override - public void release(final StringBuilder stringBuilder) { - StringBuilders.trimToMaxSize(stringBuilder, maxLength); - stringBuilder.setLength(0); - stringBuilderHolder.set(stringBuilder); - } -} diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index eedfd14acb5..b137bc540cb 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -16,20 +16,21 @@ */ package org.apache.logging.log4j.message; -import static org.apache.logging.log4j.message.ParameterFormatter.analyzePattern; - import com.google.errorprone.annotations.InlineMe; +import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; +import org.apache.logging.log4j.util.Constants; +import org.apache.logging.log4j.util.StringBuilderFormattable; +import org.apache.logging.log4j.util.internal.SerializationUtil; + import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.Arrays; import java.util.Objects; -import org.apache.logging.log4j.internal.StringBuilderRecycler; -import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; -import org.apache.logging.log4j.util.Constants; -import org.apache.logging.log4j.util.StringBuilderFormattable; -import org.apache.logging.log4j.util.internal.SerializationUtil; + +import static org.apache.logging.log4j.message.ParameterFormatter.analyzePattern; +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; /** * A {@link Message} accepting argument placeholders in the formatting pattern. @@ -79,15 +80,16 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable { private static final long serialVersionUID = -665975803997290697L; - private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( - Constants.MAX_REUSABLE_MESSAGE_SIZE, - // This value indicates the maximum recursion depth before the recycler starts creating new instances. - // Consider a `ParameterizedMessage` containing an argument such that its `toString()` causes another (i.e., - // recursive) `ParameterizedMessage` formatting. This value indicates the depth we support garbage-free - // formatting in such nested formatting situations. When this depth is exceeded, code still works, but - // starts generating garbage due to new `StringBuilder` allocations. - 3, - Constants.ENABLE_THREADLOCALS); + private static final ThreadLocal FORMAT_BUFFER_HOLDER_REF = Constants.ENABLE_THREADLOCALS + ? ThreadLocal.withInitial(FormatBufferHolder::new) + : null; + + private static final class FormatBufferHolder { + + private final StringBuilder buffer = new StringBuilder(Constants.MAX_REUSABLE_MESSAGE_SIZE); + + private boolean used = false; + } private final String pattern; @@ -244,13 +246,25 @@ public Throwable getThrowable() { @Override public String getFormattedMessage() { if (formattedMessage == null) { - final StringBuilder buffer = STRING_BUILDER_RECYCLER.acquire(); - try { - buffer.setLength(0); + final FormatBufferHolder bufferHolder; + // If there isn't a format buffer to reuse + if (FORMAT_BUFFER_HOLDER_REF == null || (bufferHolder = FORMAT_BUFFER_HOLDER_REF.get()).used) { + final StringBuilder buffer = new StringBuilder(Constants.MAX_REUSABLE_MESSAGE_SIZE); formatTo(buffer); formattedMessage = buffer.toString(); - } finally { - STRING_BUILDER_RECYCLER.release(buffer); + } + // If there is a format buffer to reuse + else { + bufferHolder.used = true; + final StringBuilder buffer = bufferHolder.buffer; + try { + formatTo(buffer); + formattedMessage = buffer.toString(); + } finally { + trimToMaxSize(buffer, Constants.MAX_REUSABLE_MESSAGE_SIZE); + buffer.setLength(0); + bufferHolder.used = false; + } } } return formattedMessage; diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java index a3b8c9cb807..f2c83947c01 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java @@ -16,13 +16,14 @@ */ package org.apache.logging.log4j.message; -import java.util.Arrays; - -import org.apache.logging.log4j.internal.StringBuilderRecycler; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.PerformanceSensitive; +import java.util.Arrays; + +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; + /** * Reusable parameterized message. This message is mutable and is not safe to be accessed or modified by multiple * threads concurrently. @@ -36,18 +37,9 @@ public class ReusableParameterizedMessage implements ReusableMessage, ParameterV private static final int MAX_PARAMS = 10; private static final long serialVersionUID = 7800075879295123856L; - private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( - Constants.MAX_REUSABLE_MESSAGE_SIZE, - // This value indicates the maximum recursion depth before the recycler starts creating new instances. - // Consider a `ParameterizedMessage` containing an argument such that its `toString()` causes another (i.e., - // recursive) `ParameterizedMessage` formatting. This value indicates the depth we support garbage-free - // formatting in such nested formatting situations. When this depth is exceeded, code still works, but - // starts generating garbage due to new `StringBuilder` allocations. - 3, - Constants.ENABLE_THREADLOCALS); - private String messagePattern; private final MessagePatternAnalysis patternAnalysis = new MessagePatternAnalysis(); + private final StringBuilder formatBuffer = new StringBuilder(Constants.MAX_REUSABLE_MESSAGE_SIZE); private int argCount; private transient Object[] varargs; private transient Object[] params = new Object[MAX_PARAMS]; @@ -345,12 +337,12 @@ public Throwable getThrowable() { */ @Override public String getFormattedMessage() { - final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); try { - formatTo(sb); - return sb.toString(); + formatTo(formatBuffer); + return formatBuffer.toString(); } finally { - STRING_BUILDER_RECYCLER.release(sb); + trimToMaxSize(formatBuffer, Constants.MAX_REUSABLE_MESSAGE_SIZE); + formatBuffer.setLength(0); } } diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java index dcb0e4d4e0e..bf5d96ddfae 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java @@ -19,7 +19,8 @@ import java.util.Iterator; import java.util.Locale; import java.util.Objects; -import org.apache.logging.log4j.internal.StringBuilderRecycler; + +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; /** * Consider this class private. @@ -29,10 +30,11 @@ @InternalApi public final class Strings { - private static final StringBuilderRecycler STRING_BUILDER_RECYCLER = StringBuilderRecycler.of( - Constants.MAX_REUSABLE_MESSAGE_SIZE, - 0, - Constants.ENABLE_THREADLOCALS); + // 518 allows the `StringBuilder` to resize three times from its initial size. + // This should be sufficient for most use cases. + private static final int MAX_FORMAT_BUFFER_LENGTH = 518; + + private static final ThreadLocal FORMAT_BUFFER_REF = ThreadLocal.withInitial(StringBuilder::new); /** * The empty string. @@ -322,11 +324,12 @@ public static String concat(final String str1, final String str2) { } else if (isEmpty(str2)) { return str1; } - final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); + final StringBuilder sb = FORMAT_BUFFER_REF.get(); try { return sb.append(str1).append(str2).toString(); } finally { - STRING_BUILDER_RECYCLER.release(sb); + trimToMaxSize(sb, MAX_FORMAT_BUFFER_LENGTH); + sb.setLength(0); } } @@ -342,14 +345,15 @@ public static String repeat(final String str, final int count) { if (count < 0) { throw new IllegalArgumentException("count"); } - final StringBuilder sb = STRING_BUILDER_RECYCLER.acquire(); + final StringBuilder sb = FORMAT_BUFFER_REF.get(); try { for (int index = 0; index < count; index++) { sb.append(str); } return sb.toString(); } finally { - STRING_BUILDER_RECYCLER.release(sb); + trimToMaxSize(sb, MAX_FORMAT_BUFFER_LENGTH); + sb.setLength(0); } } } From eacd13446e1240f8b59661c31f13c454fc2c7e21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 9 Feb 2024 20:55:55 +0100 Subject: [PATCH 4/5] Fix `ParameterizedMessageTest` --- .../logging/log4j/message/ParameterizedMessageTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java index 40e6b7b3d48..102b4039ac9 100644 --- a/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java +++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/message/ParameterizedMessageTest.java @@ -137,12 +137,12 @@ public void testSafeWithMutableParams() { // LOG4J2-763 // modify parameter before calling msg.getFormattedMessage param.set("XYZ"); final String actual = msg.getFormattedMessage(); - assertThat("Should use current param value").isEqualTo("Test message XYZ", actual); + assertThat(actual).isEqualTo("Test message XYZ").as("Should use current param value"); // modify parameter after calling msg.getFormattedMessage param.set("000"); final String after = msg.getFormattedMessage(); - assertThat("Should not change after rendered once").isEqualTo("Test message XYZ", after); + assertThat(after).isEqualTo("Test message XYZ").as("Should not change after rendered once"); } static Stream testSerializable() { From 67248ffdef08b75e304cf23fca628728325ad42a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Volkan=20Yaz=C4=B1c=C4=B1?= Date: Fri, 9 Feb 2024 21:04:12 +0100 Subject: [PATCH 5/5] Fix formatting --- .../log4j/message/ParameterizedMessage.java | 20 +++++++++---------- .../message/ReusableParameterizedMessage.java | 11 +++++----- .../apache/logging/log4j/util/Strings.java | 4 ++-- 3 files changed, 16 insertions(+), 19 deletions(-) diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java index b137bc540cb..4da3642cca1 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ParameterizedMessage.java @@ -16,21 +16,20 @@ */ package org.apache.logging.log4j.message; -import com.google.errorprone.annotations.InlineMe; -import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; -import org.apache.logging.log4j.util.Constants; -import org.apache.logging.log4j.util.StringBuilderFormattable; -import org.apache.logging.log4j.util.internal.SerializationUtil; +import static org.apache.logging.log4j.message.ParameterFormatter.analyzePattern; +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; +import com.google.errorprone.annotations.InlineMe; import java.io.IOException; import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.io.Serializable; import java.util.Arrays; import java.util.Objects; - -import static org.apache.logging.log4j.message.ParameterFormatter.analyzePattern; -import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; +import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; +import org.apache.logging.log4j.util.Constants; +import org.apache.logging.log4j.util.StringBuilderFormattable; +import org.apache.logging.log4j.util.internal.SerializationUtil; /** * A {@link Message} accepting argument placeholders in the formatting pattern. @@ -80,9 +79,8 @@ public class ParameterizedMessage implements Message, StringBuilderFormattable { private static final long serialVersionUID = -665975803997290697L; - private static final ThreadLocal FORMAT_BUFFER_HOLDER_REF = Constants.ENABLE_THREADLOCALS - ? ThreadLocal.withInitial(FormatBufferHolder::new) - : null; + private static final ThreadLocal FORMAT_BUFFER_HOLDER_REF = + Constants.ENABLE_THREADLOCALS ? ThreadLocal.withInitial(FormatBufferHolder::new) : null; private static final class FormatBufferHolder { diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java index f2c83947c01..d55b98ae700 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/message/ReusableParameterizedMessage.java @@ -16,14 +16,13 @@ */ package org.apache.logging.log4j.message; +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; + +import java.util.Arrays; import org.apache.logging.log4j.message.ParameterFormatter.MessagePatternAnalysis; import org.apache.logging.log4j.util.Constants; import org.apache.logging.log4j.util.PerformanceSensitive; -import java.util.Arrays; - -import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; - /** * Reusable parameterized message. This message is mutable and is not safe to be accessed or modified by multiple * threads concurrently. @@ -365,8 +364,8 @@ ReusableParameterizedMessage reserve() { public String toString() { // Avoid formatting arguments! // It can cause recursion, which can become pretty unpleasant while troubleshooting. - return "ReusableParameterizedMessage[messagePattern=" + getFormat() + ", argCount=" + getParameterCount() + - ", throwableProvided=" + (getThrowable() != null) + ']'; + return "ReusableParameterizedMessage[messagePattern=" + getFormat() + ", argCount=" + getParameterCount() + + ", throwableProvided=" + (getThrowable() != null) + ']'; } @Override diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java index bf5d96ddfae..bee54324f44 100644 --- a/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java +++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/Strings.java @@ -16,12 +16,12 @@ */ package org.apache.logging.log4j.util; +import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; + import java.util.Iterator; import java.util.Locale; import java.util.Objects; -import static org.apache.logging.log4j.util.StringBuilders.trimToMaxSize; - /** * Consider this class private. *