Bug 58390 - Data race on field org.apache.tomcat.util.net.SocketWrapper.error
Data race on field org.apache.tomcat.util.net.SocketWrapper.error
Status: RESOLVED FIXED
Product: Tomcat 8
Classification: Unclassified
Component: Util
8.0.x-trunk
PC Linux
: P2 normal (vote)
: ----
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2015-09-12 16:40 UTC by Yilong Li
Modified: 2015-09-17 09:03 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-12 16:40:16 UTC
Reported by RV-Predict (a dynamic race detector) when running the test suite:
Data race on field org.apache.tomcat.util.net.SocketWrapper.error: {{{
    Concurrent write in thread T35 (locks held: {})
 ---->  at org.apache.tomcat.util.net.SocketWrapper.setError(SocketWrapper.java:104)
        at org.apache.coyote.http11.upgrade.Nio2ServletOutputStream$1.failed(Nio2ServletOutputStream.java:67)
        at org.apache.tomcat.util.net.SecureNio2Channel$4.failed(SecureNio2Channel.java:844)
        at sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    T35 is created by T32
        at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:1010)

    Concurrent write in thread T36 (locks held: {})
 ---->  at org.apache.tomcat.util.net.SocketWrapper.setError(SocketWrapper.java:104)
        at org.apache.coyote.http11.upgrade.Nio2ServletInputStream$1.failed(Nio2ServletInputStream.java:70)
        at org.apache.tomcat.util.net.SecureNio2Channel$3.failed(SecureNio2Channel.java:786)
        at sun.nio.ch.AsynchronousChannelGroupImpl$1.run(AsynchronousChannelGroupImpl.java:112)
        at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    T36 is created by T32
        at java.util.concurrent.ThreadPoolExecutor.addWorker(ThreadPoolExecutor.java:1010)
}}} 

In this report, both threads are writing `true` to this field so it's not a big deal. But I am reporting it anyway in case you feel like getError and setError can be in race.
Comment 1 Mark Thomas 2015-09-17 09:03:47 UTC
The flag is only ever used by NIO2 so trunk has been refactored.

The flag is never read by NIO2 in 8.0.x so there is no issues here.

The flag was read by NIO in 8.0.x but since that read would always be false the code has been removed. (The same code was removed from trunk).

The flag is read in trunk and it has been made volatile since it is likely to be read and written in different threads.