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

Author: spevans

TestFoundation/HTTPServer.swift

@@ -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
@@ -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
+            }
         }
     }
 

TestFoundation/TestProcess.swift

@@ -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.")
     }
 
 
@@ -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()
                             }
                         }
@@ -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] {

TestFoundation/TestURLSessionFTP.swift

@@ -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

TestFoundation/Utilities.swift

@@ -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()
+    }
+}