Bug 58319 - Data race inside the non-thread-safe ArrayList org.apache.catalina.valves.TesterAccessLogValve.entries
Summary: Data race inside the non-thread-safe ArrayList org.apache.catalina.valves.Tes...
Status: RESOLVED FIXED
Alias: None
Product: Tomcat 8
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 8.0.x-trunk
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ----
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-09-03 09:50 UTC by Yilong Li
Modified: 2015-09-03 10:26 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Yilong Li 2015-09-03 09:50:47 UTC
Reported by RV-Predict (a dynamic race detector) when running the test suite:

Data race on field java.util.ArrayList.$state: {{{
Concurrent write in thread T98 (locks held: {Monitor@5e5cc762})
 ---->  at org.apache.catalina.valves.TesterAccessLogValve.log(TesterAccessLogValve.java:48)
        at org.apache.catalina.core.AccessLogAdapter.log(AccessLogAdapter.java:51)
        at org.apache.catalina.core.ContainerBase.logAccess(ContainerBase.java:1042)
        at org.apache.catalina.connector.CoyoteAdapter.event(CoyoteAdapter.java:245)
        at org.apache.coyote.http11.Http11NioProcessor.event(Http11NioProcessor.java:106)
        at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:661)
        at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1527)
        at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1484)
        - locked Monitor@5e5cc762 at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1483)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    T98 is created by T90
        at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:1010)

Concurrent read in thread T1 (locks held: {})
 ---->  at org.apache.catalina.valves.TesterAccessLogValve.validateAccessLog(TesterAccessLogValve.java:79)
        at org.apache.catalina.comet.TestCometProcessor.doSimpleCometTest(TestCometProcessor.java:347)
        at org.apache.catalina.comet.TestCometProcessor.testSimpleCometClientEndFail(TestCometProcessor.java:261)
        at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:47)
        at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
        at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:44)
        at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
        at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
        at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
        at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
        at org.junit.rules.RunRules.evaluate(RunRules.java:20)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:271)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:70)
        at org.junit.runners.ParentRunner$3.run(ParentRunner.java:238)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:63)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:236)
        at org.junit.runners.ParentRunner.access$000(ParentRunner.java:53)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:229)
        at org.junit.runners.ParentRunner.run(ParentRunner.java:309)
        at junit.framework.JUnit4TestAdapter.run(JUnit4TestAdapter.java:38)
        at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.run(JUnitTestRunner.java:532)
        at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.launch(JUnitTestRunner.java:1165)
        at org.apache.tools.ant.taskdefs.optional.junit.JUnitTestRunner.main(JUnitTestRunner.java:1016)
T1 is the main thread
}}}

Looks like TesterAccessLogValve.entries is accessed from multiple threads without proper synchronization.
Comment 1 Mark Thomas 2015-09-03 10:26:43 UTC
The way the tests are written it is extremely unlikely that this could ever be a problem. I certainly don't recall any test failures triggered by something along these lines.

Just to be safe, I've switched the ist for a ConcurrentLinkedQueue in trunk, 8.0.x and 7.0.x.