Skip to content

Commit f6be0f3

Browse files
committed
fix: fix sporadic ConcurrentModificationExceptions
Logback's ListAppender isn't thread safe unfortunately. Replace it with our own ConcurrentListAppender.
1 parent 5cf922b commit f6be0f3

File tree

3 files changed

+121
-3
lines changed

3 files changed

+121
-3
lines changed
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
package io.github.netmikey.logunit.logback;
2+
3+
import java.util.List;
4+
import java.util.Spliterator;
5+
import java.util.concurrent.ConcurrentLinkedQueue;
6+
7+
import ch.qos.logback.core.AppenderBase;
8+
import ch.qos.logback.core.read.ListAppender;
9+
10+
/**
11+
* Unfortunately, Logback's {@link ListAppender} isn't thread safe. This is a
12+
* thread-safe variant based on it. Technically speaking, it doesn't use a
13+
* {@link List} but a {@link ConcurrentLinkedQueue} and exposes items using a
14+
* {@link Splierator}.
15+
*
16+
* @See ListAppender
17+
*
18+
* @param <E>
19+
* The list element type.
20+
*/
21+
public class ConcurrentListAppender<E> extends AppenderBase<E> {
22+
23+
private ConcurrentLinkedQueue<E> list = new ConcurrentLinkedQueue<E>();
24+
25+
protected void append(E e) {
26+
list.add(e);
27+
}
28+
29+
/**
30+
* Get the items.
31+
*
32+
* @return A {@link Spliterator} over the items.
33+
*/
34+
public Spliterator<E> spliterator() {
35+
return list.spliterator();
36+
}
37+
}

logunit-logback/src/main/java/io/github/netmikey/logunit/logback/LogbackLogProvider.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import java.util.List;
66
import java.util.Map;
77
import java.util.stream.Collectors;
8+
import java.util.stream.StreamSupport;
89

910
import org.junit.jupiter.api.extension.ExtensionContext;
1011
import org.slf4j.LoggerFactory;
@@ -16,7 +17,6 @@
1617
import ch.qos.logback.classic.spi.ILoggingEvent;
1718
import ch.qos.logback.classic.spi.IThrowableProxy;
1819
import ch.qos.logback.classic.spi.ThrowableProxy;
19-
import ch.qos.logback.core.read.ListAppender;
2020
import io.github.netmikey.logunit.api.LogCapturer;
2121
import io.github.netmikey.logunit.api.LogProvider;
2222

@@ -47,7 +47,7 @@ public class LogbackLogProvider implements LogProvider {
4747
LEVEL_MAPPING_REVERSE = Collections.unmodifiableMap(levelMappingReverse);
4848
}
4949

50-
private final ListAppender<ILoggingEvent> listAppender = new ListAppender<ILoggingEvent>();
50+
private final ConcurrentListAppender<ILoggingEvent> listAppender = new ConcurrentListAppender<ILoggingEvent>();
5151

5252
private final Map<Class<?>, Level> loggerTypes = new HashMap<>();
5353

@@ -75,7 +75,9 @@ public void provideForLogger(String name, org.slf4j.event.Level level) {
7575

7676
@Override
7777
public List<LoggingEvent> getEvents() {
78-
return listAppender.list.stream().map(this::mapEvent).collect(Collectors.toList());
78+
return StreamSupport.stream(listAppender.spliterator(), false)
79+
.map(this::mapEvent)
80+
.collect(Collectors.toList());
7981
}
8082

8183
@Override
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
package io.github.netmikey.logunit.logback;
2+
3+
import java.util.ConcurrentModificationException;
4+
5+
import org.junit.jupiter.api.AfterEach;
6+
import org.junit.jupiter.api.BeforeEach;
7+
import org.junit.jupiter.api.RepeatedTest;
8+
import org.junit.jupiter.api.Test;
9+
import org.slf4j.LoggerFactory;
10+
import org.slf4j.event.Level;
11+
12+
import ch.qos.logback.core.read.ListAppender;
13+
14+
/**
15+
* There have been sightings of {@link ConcurrentModificationException}s caused
16+
* by Logback's {@link ListAppender}. This test aims at reproducing this and
17+
* testing the implemented {@link ConcurrentListAppender} solution.
18+
*/
19+
public class LogbackLogProviderConcurrencyTest {
20+
21+
private static final org.slf4j.Logger LOGGER = LoggerFactory.getLogger(LogbackLogProviderConcurrencyTest.class);
22+
23+
private static final int LOG_COUNT = 100;
24+
25+
private LogbackLogProvider tested;
26+
27+
private Thread loggingThread;
28+
29+
/**
30+
* JUnit's beforeEach method.
31+
*/
32+
@BeforeEach
33+
public void beforeEach() {
34+
// Prepare unit under test
35+
tested = new LogbackLogProvider();
36+
tested.provideForLogger(LOGGER.getName(), Level.TRACE);
37+
tested.beforeTestExecution(null);
38+
39+
// Prepare a thread that is going to write logs
40+
loggingThread = new Thread(() -> {
41+
for (int i = 0; i < LOG_COUNT; i++) {
42+
LOGGER.trace("It makes BOOM!");
43+
}
44+
});
45+
}
46+
47+
/**
48+
* JUnit's afterEach method.
49+
*
50+
* @throws Exception
51+
* An unexpected exception occured.
52+
*/
53+
@AfterEach
54+
public void afterEach() throws Exception {
55+
// Wait for the thread to terminate
56+
loggingThread.join();
57+
58+
// Cleanup
59+
tested.afterTestExecution(null);
60+
}
61+
62+
/**
63+
* Test the {@link LogbackLogProvider} for concurrency.
64+
*
65+
* @throws Exception
66+
* An unexpected exception occurred.
67+
*/
68+
@Test
69+
@RepeatedTest(value = 15)
70+
public void testConcurrency() throws Exception {
71+
// Start the logging thread, and...
72+
loggingThread.start();
73+
74+
// ... try to fetc log events concurrently
75+
for (int i = 0; i < LOG_COUNT; i++) {
76+
tested.getEvents();
77+
}
78+
}
79+
}

0 commit comments

Comments
 (0)