Skip to content

Commit 32d29ae

Browse files
ppkarwaszppkarwasz
authored
fix: correctly detect Disruptor major version (#3778)
### fix(test): fail `DisruptorTest` on async thread exceptions Ensure that DisruptorTest explicitly fails when an exception occurs on an asynchronous thread. This improves error detection and prevents silent test passes in the presence of async failures. ### fix: correctly detect Disruptor major version Ensure the Disruptor version is detected using the classloader that loaded `DisruptorUtil`, rather than the thread-context classloader. The previous implementation relied on `LoaderUtil.isClassAvailable`, which may fail in environments where the Disruptor classes aren't visible to the thread-context classloader.
1 parent e08c7ba commit 32d29ae

File tree

4 files changed

+112
-17
lines changed

4 files changed

+112
-17
lines changed

‎log4j-core/src/main/java/org/apache/logging/log4j/core/async/DisruptorUtil.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,19 @@ final class DisruptorUtil {
5050
static final boolean ASYNC_CONFIG_SYNCHRONIZE_ENQUEUE_WHEN_QUEUE_FULL = PropertiesUtil.getProperties()
5151
.getBooleanProperty("AsyncLoggerConfig.SynchronizeEnqueueWhenQueueFull", true);
5252

53-
static final int DISRUPTOR_MAJOR_VERSION =
54-
LoaderUtil.isClassAvailable("com.lmax.disruptor.SequenceReportingEventHandler") ? 3 : 4;
53+
static final int DISRUPTOR_MAJOR_VERSION = detectDisruptorMajorVersion();
54+
55+
// TODO: replace with LoaderUtil.isClassAvailable() when TCCL is removed
56+
// See: https://github.com/apache/logging-log4j2/issues/3706
57+
private static int detectDisruptorMajorVersion() {
58+
try {
59+
Class.forName(
60+
"com.lmax.disruptor.SequenceReportingEventHandler", true, DisruptorUtil.class.getClassLoader());
61+
return 3;
62+
} catch (final ClassNotFoundException e) {
63+
return 4;
64+
}
65+
}
5566

5667
private DisruptorUtil() {}
5768

‎log4j-osgi-test/pom.xml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@
257257
<include>org.apache.logging.log4j.osgi.tests.DisruptorTest</include>
258258
</includes>
259259
<systemPropertyVariables>
260+
<log4j2.asyncLoggerExceptionHandler>org.apache.logging.log4j.osgi.tests.DisruptorTest$TestExceptionHandler</log4j2.asyncLoggerExceptionHandler>
260261
<log4j2.contextSelector>org.apache.logging.log4j.core.async.BasicAsyncLoggerContextSelector</log4j2.contextSelector>
261262
</systemPropertyVariables>
262263
</configuration>
@@ -291,6 +292,7 @@
291292
<include>org.apache.logging.log4j.osgi.tests.DisruptorTest</include>
292293
</includes>
293294
<systemPropertyVariables>
295+
<log4j2.asyncLoggerExceptionHandler>org.apache.logging.log4j.osgi.tests.DisruptorTest$TestExceptionHandler</log4j2.asyncLoggerExceptionHandler>
294296
<log4j2.contextSelector>org.apache.logging.log4j.core.async.BasicAsyncLoggerContextSelector</log4j2.contextSelector>
295297
</systemPropertyVariables>
296298
</configuration>

‎log4j-osgi-test/src/test/java/org/apache/logging/log4j/osgi/tests/DisruptorTest.java

Lines changed: 85 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,26 @@
1717
package org.apache.logging.log4j.osgi.tests;
1818

1919
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNull;
2021
import static org.junit.Assert.assertTrue;
2122
import static org.ops4j.pax.exam.CoreOptions.junitBundles;
2223
import static org.ops4j.pax.exam.CoreOptions.linkBundle;
2324
import static org.ops4j.pax.exam.CoreOptions.options;
2425

26+
import com.lmax.disruptor.ExceptionHandler;
27+
import java.io.IOException;
28+
import java.net.URL;
29+
import java.util.concurrent.atomic.AtomicReference;
2530
import org.apache.logging.log4j.Level;
2631
import org.apache.logging.log4j.LogManager;
2732
import org.apache.logging.log4j.Logger;
2833
import org.apache.logging.log4j.core.LogEvent;
2934
import org.apache.logging.log4j.core.LoggerContext;
3035
import org.apache.logging.log4j.core.async.AsyncLoggerContext;
36+
import org.apache.logging.log4j.core.async.RingBufferLogEvent;
3137
import org.junit.Test;
3238
import org.junit.runner.RunWith;
39+
import org.junitpioneer.jupiter.Issue;
3340
import org.ops4j.pax.exam.Option;
3441
import org.ops4j.pax.exam.junit.PaxExam;
3542
import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
@@ -39,6 +46,8 @@
3946
@ExamReactorStrategy(PerClass.class)
4047
public class DisruptorTest {
4148

49+
private static final int MESSAGE_COUNT = 128;
50+
4251
@org.ops4j.pax.exam.Configuration
4352
public Option[] config() {
4453
return options(
@@ -59,26 +68,87 @@ public Option[] config() {
5968
}
6069

6170
@Test
62-
public void testDisruptorLog() {
63-
// Logger context
64-
LoggerContext context = getLoggerContext();
65-
assertTrue("LoggerContext is an instance of AsyncLoggerContext", context instanceof AsyncLoggerContext);
66-
final CustomConfiguration custom = (CustomConfiguration) context.getConfiguration();
67-
// Logging
68-
final Logger logger = LogManager.getLogger(getClass());
69-
logger.info("Hello OSGI from Log4j2!");
70-
71-
context.stop();
72-
assertEquals(1, custom.getEvents().size());
73-
final LogEvent event = custom.getEvents().get(0);
74-
assertEquals("Hello OSGI from Log4j2!", event.getMessage().getFormattedMessage());
75-
assertEquals(Level.INFO, event.getLevel());
76-
custom.clearEvents();
71+
@Issue("https://github.com/apache/logging-log4j2/issues/3706")
72+
public void testDisruptorLog() throws IOException {
73+
ClassLoader threadContextClassLoader = Thread.currentThread().getContextClassLoader();
74+
ClassLoader classLoader = createClassLoader();
75+
try {
76+
// Set the context classloader to an empty classloader, so attempts to use the TCCL will not find any
77+
// classes.
78+
Thread.currentThread().setContextClassLoader(classLoader);
79+
// Logger context
80+
LoggerContext context = getLoggerContext();
81+
assertTrue("LoggerContext is an instance of AsyncLoggerContext", context instanceof AsyncLoggerContext);
82+
final CustomConfiguration custom = (CustomConfiguration) context.getConfiguration();
83+
// Logging
84+
final Logger logger = LogManager.getLogger(getClass());
85+
for (int i = 0; i < MESSAGE_COUNT; i++) {
86+
logger.info("Hello OSGI from Log4j2! {}", i);
87+
}
88+
89+
context.stop();
90+
assertEquals(MESSAGE_COUNT, custom.getEvents().size());
91+
for (int i = 0; i < MESSAGE_COUNT; i++) {
92+
final LogEvent event = custom.getEvents().get(i);
93+
assertEquals(
94+
"Message nr " + i,
95+
"Hello OSGI from Log4j2! " + i,
96+
event.getMessage().getFormattedMessage());
97+
assertEquals(Level.INFO, event.getLevel());
98+
}
99+
custom.clearEvents();
100+
assertNull("Asynchronous exception", TestExceptionHandler.exception.get());
101+
} finally {
102+
Thread.currentThread().setContextClassLoader(threadContextClassLoader);
103+
}
104+
}
105+
106+
private static ClassLoader createClassLoader() {
107+
// We want a classloader capable only of loading TestExceptionHandler.
108+
// This is needed to detect exceptions thrown by the asynchronous thread.
109+
return new ClassLoader() {
110+
@Override
111+
public Class<?> loadClass(String name) throws ClassNotFoundException {
112+
if (name.equals(TestExceptionHandler.class.getName())) {
113+
return TestExceptionHandler.class;
114+
}
115+
throw new ClassNotFoundException(name);
116+
}
117+
118+
@Override
119+
public URL getResource(String name) {
120+
return null; // No resources available.
121+
}
122+
};
77123
}
78124

79125
private static LoggerContext getLoggerContext() {
80126
final LoggerContext ctx = (LoggerContext) LogManager.getContext(false);
81127
assertEquals("AsyncDefault", ctx.getName());
82128
return ctx;
83129
}
130+
131+
public static class TestExceptionHandler implements ExceptionHandler<RingBufferLogEvent> {
132+
133+
private static final AtomicReference<Throwable> exception = new AtomicReference<>();
134+
135+
@Override
136+
public void handleEventException(Throwable ex, long sequence, RingBufferLogEvent event) {
137+
setException(ex);
138+
}
139+
140+
@Override
141+
public void handleOnStartException(Throwable ex) {
142+
setException(ex);
143+
}
144+
145+
@Override
146+
public void handleOnShutdownException(Throwable ex) {
147+
setException(ex);
148+
}
149+
150+
private static void setException(Throwable ex) {
151+
exception.compareAndSet(null, ex);
152+
}
153+
}
84154
}

‎src/changelog/.2.x.x/3706_disruptor-tccl.xml

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
<?xml version="1.0" encoding="UTF-8"?>
2+
<entry xmlns="https://logging.apache.org/xml/ns"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="
5+
https://logging.apache.org/xml/ns
6+
https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
7+
type="fixed">
8+
<issue id="3706" link="https://github.com/apache/logging-log4j2/issues/3706"/>
9+
<description format="asciidoc">
10+
Fix detection of the Disruptor major version in some environments.
11+
</description>
12+
</entry>

0 commit comments

Comments
 (0)