signalapp/Starscream
signalapp/Starscream
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid TSan data race with CompressionState

Browse Source 
Most of CompressionState is only mutated once per connection, as
part of setting up the websocket.

But needsCompression is mutated per-read-message.

Handling reads is serialized on one queue, and handling writes is serialized on
another queueu, which means that a read can be handled concurrently with
handling a write.

In theory this concurrent read/write should be harmless because the mutated
field is not accessed by the other queue, but for best practice, we should
write our code in a way that keeps TSan happy.
This commit is contained in:
Michael Kirk 2019-02-05 10:20:15 -07:00
parent 40cda44e40
commit 1642cbda48
1 changed files with 7 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
Sources/Starscream/WebSocket.swift
View File

@ -433,7 +433,6 @@ open class WebSocket : NSObject, StreamDelegate, WebSocketClient, WSStreamDelega
private struct CompressionState {
var supportsCompression = false
var messageNeedsDecompression = false
var serverMaxWindowBits = 15
var clientMaxWindowBits = 15
var clientNoContextTakeover = false
@ -447,6 +446,10 @@ open class WebSocket : NSObject, StreamDelegate, WebSocketClient, WSStreamDelega
private var isConnecting = false
private let mutex = NSLock()
private var compressionState = CompressionState()
// `currentMessageNeedsDecompression` is not part of the `compressionState` struct
// because currentMessageNeedsDecompression can be mutated in the read queue concurrently
// with other `compressionState` fields being accesseed on the write queue.
private var currentMessageNeedsDecompression = false
private var writeQueue = OperationQueue()
private var readStack = [WSResponse]()
private var inputQueue = [Data]()
@ -996,9 +999,9 @@ open class WebSocket : NSObject, StreamDelegate, WebSocketClient, WSStreamDelega
let payloadLen = (PayloadLenMask & baseAddress[1])
var offset = 2
if compressionState.supportsCompression && receivedOpcode != .continueFrame {
compressionState.messageNeedsDecompression = (RSV1Mask & baseAddress[0]) > 0
currentMessageNeedsDecompression = (RSV1Mask & baseAddress[0]) > 0
}
if (isMasked > 0 || (RSVMask & baseAddress[0]) > 0) && receivedOpcode != .pong && !compressionState.messageNeedsDecompression {
if (isMasked > 0 || (RSVMask & baseAddress[0]) > 0) && receivedOpcode != .pong && !currentMessageNeedsDecompression {
let errCode = CloseCode.protocolError.rawValue
doDisconnect(WSError(type: .protocolError, message: "masked and rsv data is not currently supported", code: Int(errCode)))
writeError(errCode)
@ -1059,7 +1062,7 @@ open class WebSocket : NSObject, StreamDelegate, WebSocketClient, WSStreamDelega
len -= UInt64(size)
}
let data: Data
if compressionState.messageNeedsDecompression, let decompressor = compressionState.decompressor {
if currentMessageNeedsDecompression, let decompressor = compressionState.decompressor {
do {
data = try decompressor.decompress(bytes: baseAddress+offset, count: Int(len), finish: isFin > 0)
if isFin > 0 && compressionState.serverNoContextTakeover {