From f1ef2a2643ec593181e23618ba2a01bb941a0bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20L=C3=BCck?= Date: Thu, 8 May 2025 13:18:12 +0200 Subject: [PATCH 1/3] Handle missing stack traces in ExtendedThreadInformation Fix ArrayIndexOutOfBoundsException on invocation of Message.getFormattedMessage() when any thread has no stack trace, which occurs on some JVM implementations. --- .../ExtendedThreadInformationTest.java | 45 +++++++++++++++++++ .../message/ExtendedThreadInformation.java | 14 +++++- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/message/ExtendedThreadInformationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/message/ExtendedThreadInformationTest.java index 3dca41c498f..95351959f81 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/message/ExtendedThreadInformationTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/message/ExtendedThreadInformationTest.java @@ -16,10 +16,17 @@ */ package org.apache.logging.log4j.core.message; +import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import java.lang.management.ThreadInfo; import org.apache.logging.log4j.message.ThreadDumpMessage; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; /** * Tests that ThreadDumpMessage uses ExtendedThreadInformation when available. @@ -33,4 +40,42 @@ void testMessage() { // System.out.print(message); assertTrue(message.contains(" Id="), "No header"); } + + @ParameterizedTest + @EnumSource(Thread.State.class) + void testMessageWithNullStackTrace(final Thread.State state) { + obtainMessageWithMissingStackTrace(state, null); + } + + @ParameterizedTest + @EnumSource(Thread.State.class) + void testMessageWithEmptyStackTrace(final Thread.State state) { + obtainMessageWithMissingStackTrace(state, new StackTraceElement[0]); + } + + private void obtainMessageWithMissingStackTrace(final Thread.State state, final StackTraceElement[] stackTrace) { + // setup + final String threadName = "the thread name"; + final long threadId = 23523L; + + final ThreadInfo threadInfo = mock(ThreadInfo.class); + when(threadInfo.getStackTrace()).thenReturn(stackTrace); + when(threadInfo.getThreadName()).thenReturn(threadName); + when(threadInfo.getThreadId()).thenReturn(threadId); + when(threadInfo.isSuspended()).thenReturn(true); + when(threadInfo.isInNative()).thenReturn(true); + when(threadInfo.getThreadState()).thenReturn(state); + + // given + final ExtendedThreadInformation sut = new ExtendedThreadInformation(threadInfo); + + // when + final StringBuilder result = new StringBuilder(); + sut.printThreadInfo(result); + + // then + assertThat(result.toString(), containsString(threadName)); + assertThat(result.toString(), containsString(state.name())); + assertThat(result.toString(), containsString(String.valueOf(threadId))); + } } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java index c6ea59d1c64..bc9cfda642b 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java @@ -119,7 +119,7 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { break; } case WAITING: { - final StackTraceElement element = info.getStackTrace()[0]; + final StackTraceElement element = getStackTraceElement(info); final String className = element.getClassName(); final String method = element.getMethodName(); if (className.equals("java.lang.Object") && method.equals("wait")) { @@ -144,7 +144,7 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { break; } case TIMED_WAITING: { - final StackTraceElement element = info.getStackTrace()[0]; + final StackTraceElement element = getStackTraceElement(info); final String className = element.getClassName(); final String method = element.getMethodName(); if (className.equals("java.lang.Object") && method.equals("wait")) { @@ -174,4 +174,14 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { break; } } + + private static StackTraceElement getStackTraceElement(final ThreadInfo info) { + final StackTraceElement[] stackTrace = info.getStackTrace(); + if (stackTrace == null || stackTrace.length < 1) { + final String textualInfo = "Unknown"; + return new StackTraceElement(textualInfo, textualInfo, textualInfo, -1); + } + + return stackTrace[0]; + } } From 0907df1f23df414ae0b3bc0c0780f2f84a809dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20L=C3=BCck?= Date: Thu, 8 May 2025 14:43:35 +0200 Subject: [PATCH 2/3] Add release notes file --- ...ssing_stack_traces_in_ExtendedThreadInformation.xml | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 src/changelog/.2.x.x/3655_handle_missing_stack_traces_in_ExtendedThreadInformation.xml diff --git a/src/changelog/.2.x.x/3655_handle_missing_stack_traces_in_ExtendedThreadInformation.xml b/src/changelog/.2.x.x/3655_handle_missing_stack_traces_in_ExtendedThreadInformation.xml new file mode 100644 index 00000000000..738ae8bef5b --- /dev/null +++ b/src/changelog/.2.x.x/3655_handle_missing_stack_traces_in_ExtendedThreadInformation.xml @@ -0,0 +1,10 @@ + + + + + Fix `ArrayIndexOutOfBoundsException` on invocation of `Message.getFormattedMessage()` when any thread has no stack trace, which occurs on some JVM implementations. + + From 5584f8c3257e02b0e208423ae002c811f0cb7747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20L=C3=BCck?= Date: Mon, 12 May 2025 07:03:30 +0200 Subject: [PATCH 3/3] Refactor thread info handling to improve null safety. Updated `getStackTraceElement` to return null instead of a placeholder when stack trace is empty. Added null checks to ensure safety when accessing class name and method name, and replaced string comparisons with null-safe constants. $ Please enter the commit message for your changes. Lines starting $ with '$' will be kept; you may remove them yourself if you want to. $ An empty message aborts the commit. $ $ Date: Mon May 12 07:03:30 2025 +0200 $ $ On branch 2.x $ Your branch is up to date with 'origin/2.x'. $ $ Changes to be committed: $ modified: log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java $ --- .../message/ExtendedThreadInformation.java | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java index bc9cfda642b..d3ea79e7ffb 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/message/ExtendedThreadInformation.java @@ -23,6 +23,7 @@ import java.lang.management.ThreadInfo; import org.apache.logging.log4j.message.ThreadInformation; import org.apache.logging.log4j.util.StringBuilders; +import org.jspecify.annotations.Nullable; /** * Provides information on locks and monitors in the thread dump. This class requires Java 1.6 to compile and @@ -119,17 +120,17 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { break; } case WAITING: { - final StackTraceElement element = getStackTraceElement(info); - final String className = element.getClassName(); - final String method = element.getMethodName(); - if (className.equals("java.lang.Object") && method.equals("wait")) { + final StackTraceElement element = getFirstStackTraceElement(info); + final String className = element != null ? element.getClassName() : null; + final String method = element != null ? element.getMethodName() : null; + if ("java.lang.Object".equals(className) && "wait".equals(method)) { sb.append(" (on object monitor"); if (info.getLockOwnerName() != null) { sb.append(" owned by \""); sb.append(info.getLockOwnerName()).append("\" Id=").append(info.getLockOwnerId()); } sb.append(')'); - } else if (className.equals("java.lang.Thread") && method.equals("join")) { + } else if ("java.lang.Thread".equals(className) && "join".equals(method)) { sb.append(" (on completion of thread ") .append(info.getLockOwnerId()) .append(')'); @@ -144,19 +145,19 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { break; } case TIMED_WAITING: { - final StackTraceElement element = getStackTraceElement(info); - final String className = element.getClassName(); - final String method = element.getMethodName(); - if (className.equals("java.lang.Object") && method.equals("wait")) { + final StackTraceElement element = getFirstStackTraceElement(info); + final String className = element != null ? element.getClassName() : null; + final String method = element != null ? element.getMethodName() : null; + if ("java.lang.Object".equals(className) && "wait".equals(method)) { sb.append(" (on object monitor"); if (info.getLockOwnerName() != null) { sb.append(" owned by \""); sb.append(info.getLockOwnerName()).append("\" Id=").append(info.getLockOwnerId()); } sb.append(')'); - } else if (className.equals("java.lang.Thread") && method.equals("sleep")) { + } else if ("java.lang.Thread".equals(className) && "sleep".equals(method)) { sb.append(" (sleeping)"); - } else if (className.equals("java.lang.Thread") && method.equals("join")) { + } else if ("java.lang.Thread".equals(className) && "join".equals(method)) { sb.append(" (on completion of thread ") .append(info.getLockOwnerId()) .append(')'); @@ -175,13 +176,9 @@ private void formatState(final StringBuilder sb, final ThreadInfo info) { } } - private static StackTraceElement getStackTraceElement(final ThreadInfo info) { + @Nullable + private static StackTraceElement getFirstStackTraceElement(final ThreadInfo info) { final StackTraceElement[] stackTrace = info.getStackTrace(); - if (stackTrace == null || stackTrace.length < 1) { - final String textualInfo = "Unknown"; - return new StackTraceElement(textualInfo, textualInfo, textualInfo, -1); - } - - return stackTrace[0]; + return stackTrace != null && stackTrace.length > 0 ? stackTrace[0] : null; } }