Skip to content

Commit 4a288af

Browse files
committed
Avoid concurrent modification in mock log appender (elastic#41424)
It can be the case that while we are setting up expectations that also a log message is appended. For example, if we are setting up these expectations after a cluster has formed and messages start being sent around the cluster. In this case, we would hit a concurrent modification exception while we are mutating the expectations, and also while the expectations are being iterated over as a message is appended. This commit avoids this by using a copy-on-write array list which is safe for concurrent modification and iteration. Note that another possible approach here is to use synchronized, but that seems unnecessary since we don't appear to rely on messages that are sent while we are setting up expectations. Rather, we are setting up some expectations and some situation that we think will cause those expectations to be met. Using copy-on-write array list here is nice since we avoid bottlenecking these tests on synchronizing these methods.
1 parent e6c24e3 commit 4a288af

File tree

2 files changed

+7
-3
lines changed

2 files changed

+7
-3
lines changed

test/framework/src/main/java/org/elasticsearch/test/MockLogAppender.java

+7-2
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
import org.apache.logging.log4j.core.filter.RegexFilter;
2525
import org.elasticsearch.common.regex.Regex;
2626

27-
import java.util.ArrayList;
2827
import java.util.List;
28+
import java.util.concurrent.CopyOnWriteArrayList;
2929
import java.util.regex.Pattern;
3030

3131
import static org.hamcrest.CoreMatchers.equalTo;
@@ -42,7 +42,12 @@ public class MockLogAppender extends AbstractAppender {
4242

4343
public MockLogAppender() throws IllegalAccessException {
4444
super("mock", RegexFilter.createFilter(".*(\n.*)*", new String[0], false, null, null), null);
45-
expectations = new ArrayList<>();
45+
/*
46+
* We use a copy-on-write array list since log messages could be appended while we are setting up expectations. When that occurs,
47+
* we would run into a concurrent modification exception from the iteration over the expectations in #append, concurrent with a
48+
* modification from #addExpectation.
49+
*/
50+
expectations = new CopyOnWriteArrayList<>();
4651
}
4752

4853
public void addExpectation(LoggingExpectation expectation) {

test/framework/src/main/java/org/elasticsearch/transport/AbstractSimpleTransportTestCase.java

-1
Original file line numberDiff line numberDiff line change
@@ -993,7 +993,6 @@ public void handleException(TransportException exp) {
993993
}
994994

995995
@TestLogging(value = "org.elasticsearch.transport.TransportService.tracer:trace")
996-
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/40586")
997996
public void testTracerLog() throws Exception {
998997
TransportRequestHandler<TransportRequest> handler = (request, channel, task) -> channel.sendResponse(new StringMessageResponse(""));
999998
TransportRequestHandler<StringMessageRequest> handlerWithError = (request, channel, task) -> {

0 commit comments

Comments
 (0)