Skip to content

TSan: Fix some data race issues found in the tests by Thread Sanitizer. #2523

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 13 additions & 6 deletions TestFoundation/HTTPServer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ class _TCPSocket {
private let sendFlags: CInt
#endif
private var listenSocket: SOCKET!
private var socketAddress = UnsafeMutablePointer<sockaddr_in>.allocate(capacity: 1)
private var connectionSocket: SOCKET?
private var socketAddress = UnsafeMutablePointer<sockaddr_in>.allocate(capacity: 1)
private var _connectionSocketLock = NSLock()
private var _connectionSocket: SOCKET?
private var connectionSocket: SOCKET? {
get { _connectionSocketLock.synchronized { _connectionSocket } }
set { _connectionSocketLock.synchronized { _connectionSocket = newValue } }
}

private func isNotNegative(r: CInt) -> Bool {
return r != -1
Expand Down Expand Up @@ -229,13 +234,15 @@ class _TCPSocket {
}

func closeClient() {
if let connectionSocket = self.connectionSocket {
_connectionSocketLock.synchronized {
if let connectionSocket = _connectionSocket {
#if os(Windows)
closesocket(connectionSocket)
closesocket(connectionSocket)
#else
close(connectionSocket)
close(connectionSocket)
#endif
self.connectionSocket = nil
_connectionSocket = nil
}
}
}

Expand Down
74 changes: 46 additions & 28 deletions TestFoundation/TestProcess.swift
Original file line number Diff line number Diff line change
Expand Up @@ -550,19 +550,30 @@ class TestProcess : XCTestCase {
task.executableURL = url
task.arguments = []
let stdoutPipe = Pipe()
let dataLock = NSLock()
task.standardOutput = stdoutPipe

var stdoutData = Data()
stdoutPipe.fileHandleForReading.readabilityHandler = { fh in
stdoutData.append(fh.availableData)
dataLock.synchronized {
stdoutData.append(fh.availableData)
}
}
try task.run()
task.waitUntilExit()
stdoutPipe.fileHandleForReading.readabilityHandler = nil
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
stdoutData.append(d)

try dataLock.synchronized {
#if DARWIN_COMPATIBILITY_TESTS
// Use old API for now
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
#else
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
stdoutData.append(d)
}
#endif
XCTAssertEqual(String(data: stdoutData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines), "No files specified.")
}
XCTAssertEqual(String(data: stdoutData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines), "No files specified.")
}


Expand Down Expand Up @@ -656,11 +667,11 @@ class _SignalHelperRunner {
}
else if strongSelf.gotReady == true {
if line == "Signal: SIGINT" {
strongSelf._sigIntCount += 1
strongSelf.sQueue.sync { strongSelf._sigIntCount += 1 }
strongSelf.semaphore.signal()
}
else if line == "Signal: SIGCONT" {
strongSelf._sigContCount += 1
strongSelf.sQueue.sync { strongSelf._sigContCount += 1 }
strongSelf.semaphore.signal()
}
}
Expand Down Expand Up @@ -717,52 +728,59 @@ internal func runTask(_ arguments: [String], environment: [String: String]? = ni

let stdoutPipe = Pipe()
let stderrPipe = Pipe()
let dataLock = NSLock()
process.standardOutput = stdoutPipe
process.standardError = stderrPipe

var stdoutData = Data()
stdoutPipe.fileHandleForReading.readabilityHandler = { fh in
stdoutData.append(fh.availableData)
dataLock.synchronized {
stdoutData.append(fh.availableData)
}
}

var stderrData = Data()
stderrPipe.fileHandleForReading.readabilityHandler = { fh in
stderrData.append(fh.availableData)
dataLock.synchronized {
stderrData.append(fh.availableData)
}
}

try process.run()
process.waitUntilExit()
stdoutPipe.fileHandleForReading.readabilityHandler = nil
stderrPipe.fileHandleForReading.readabilityHandler = nil

// Drain any data remaining in the pipes
guard process.terminationStatus == 0 else {
throw Error.TerminationStatus(process.terminationStatus)
}

return try dataLock.synchronized {
// Drain any data remaining in the pipes
#if DARWIN_COMPATIBILITY_TESTS
// Use old API for now
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
stderrData.append(stderrPipe.fileHandleForReading.availableData)
// Use old API for now
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
stderrData.append(stderrPipe.fileHandleForReading.availableData)
#else
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
stdoutData.append(d)
}
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
stdoutData.append(d)
}

if let d = try stderrPipe.fileHandleForReading.readToEnd() {
stderrData.append(d)
}
if let d = try stderrPipe.fileHandleForReading.readToEnd() {
stderrData.append(d)
}
#endif

guard process.terminationStatus == 0 else {
throw Error.TerminationStatus(process.terminationStatus)
}
guard let stdout = String(data: stdoutData, encoding: .utf8) else {
throw Error.UnicodeDecodingError(stdoutData)
}

guard let stdout = String(data: stdoutData, encoding: .utf8) else {
throw Error.UnicodeDecodingError(stdoutData)
}
guard let stderr = String(data: stderrData, encoding: .utf8) else {
throw Error.UnicodeDecodingError(stderrData)
}

guard let stderr = String(data: stderrData, encoding: .utf8) else {
throw Error.UnicodeDecodingError(stderrData)
return (stdout, stderr)
}

return (stdout, stderr)
}

private func parseEnv(_ env: String) throws -> [String: String] {
Expand Down
8 changes: 7 additions & 1 deletion TestFoundation/TestURLSessionFTP.swift
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,13 @@ class FTPDataTask : NSObject {
var cancelExpectation: XCTestExpectation?
var responseReceivedExpectation: XCTestExpectation?
var hasTransferCompleted = false
public var error = false

private var errorLock = NSLock()
private var _error = false
public var error: Bool {
get { errorLock.synchronized { _error } }
set { errorLock.synchronized { _error = newValue } }
}

init(with expectation: XCTestExpectation) {
dataTaskExpectation = expectation
Expand Down
7 changes: 7 additions & 0 deletions TestFoundation/Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -641,3 +641,10 @@ extension FileHandle: TextOutputStream {
}
}

extension NSLock {
public func synchronized<T>(_ closure: () throws -> T) rethrows -> T {
self.lock()
defer { self.unlock() }
return try closure()
}
}