Skip to content

Commit 7019717

Browse files
authored
Merge pull request #2523 from spevans/pr_tsan_fixes
2 parents eca9e18 + 82a5aae commit 7019717

File tree

4 files changed

+73
-35
lines changed

4 files changed

+73
-35
lines changed

TestFoundation/HTTPServer.swift

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,13 @@ class _TCPSocket {
5151
private let sendFlags: CInt
5252
#endif
5353
private var listenSocket: SOCKET!
54-
private var socketAddress = UnsafeMutablePointer<sockaddr_in>.allocate(capacity: 1)
55-
private var connectionSocket: SOCKET?
54+
private var socketAddress = UnsafeMutablePointer<sockaddr_in>.allocate(capacity: 1)
55+
private var _connectionSocketLock = NSLock()
56+
private var _connectionSocket: SOCKET?
57+
private var connectionSocket: SOCKET? {
58+
get { _connectionSocketLock.synchronized { _connectionSocket } }
59+
set { _connectionSocketLock.synchronized { _connectionSocket = newValue } }
60+
}
5661

5762
private func isNotNegative(r: CInt) -> Bool {
5863
return r != -1
@@ -229,13 +234,15 @@ class _TCPSocket {
229234
}
230235

231236
func closeClient() {
232-
if let connectionSocket = self.connectionSocket {
237+
_connectionSocketLock.synchronized {
238+
if let connectionSocket = _connectionSocket {
233239
#if os(Windows)
234-
closesocket(connectionSocket)
240+
closesocket(connectionSocket)
235241
#else
236-
close(connectionSocket)
242+
close(connectionSocket)
237243
#endif
238-
self.connectionSocket = nil
244+
_connectionSocket = nil
245+
}
239246
}
240247
}
241248

TestFoundation/TestProcess.swift

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -557,19 +557,30 @@ class TestProcess : XCTestCase {
557557
task.executableURL = url
558558
task.arguments = []
559559
let stdoutPipe = Pipe()
560+
let dataLock = NSLock()
560561
task.standardOutput = stdoutPipe
561562

562563
var stdoutData = Data()
563564
stdoutPipe.fileHandleForReading.readabilityHandler = { fh in
564-
stdoutData.append(fh.availableData)
565+
dataLock.synchronized {
566+
stdoutData.append(fh.availableData)
567+
}
565568
}
566569
try task.run()
567570
task.waitUntilExit()
568571
stdoutPipe.fileHandleForReading.readabilityHandler = nil
569-
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
570-
stdoutData.append(d)
572+
573+
try dataLock.synchronized {
574+
#if DARWIN_COMPATIBILITY_TESTS
575+
// Use old API for now
576+
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
577+
#else
578+
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
579+
stdoutData.append(d)
580+
}
581+
#endif
582+
XCTAssertEqual(String(data: stdoutData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines), "No files specified.")
571583
}
572-
XCTAssertEqual(String(data: stdoutData, encoding: .utf8)?.trimmingCharacters(in: .whitespacesAndNewlines), "No files specified.")
573584
}
574585

575586

@@ -663,11 +674,11 @@ class _SignalHelperRunner {
663674
}
664675
else if strongSelf.gotReady == true {
665676
if line == "Signal: SIGINT" {
666-
strongSelf._sigIntCount += 1
677+
strongSelf.sQueue.sync { strongSelf._sigIntCount += 1 }
667678
strongSelf.semaphore.signal()
668679
}
669680
else if line == "Signal: SIGCONT" {
670-
strongSelf._sigContCount += 1
681+
strongSelf.sQueue.sync { strongSelf._sigContCount += 1 }
671682
strongSelf.semaphore.signal()
672683
}
673684
}
@@ -724,52 +735,59 @@ internal func runTask(_ arguments: [String], environment: [String: String]? = ni
724735

725736
let stdoutPipe = Pipe()
726737
let stderrPipe = Pipe()
738+
let dataLock = NSLock()
727739
process.standardOutput = stdoutPipe
728740
process.standardError = stderrPipe
729741

730742
var stdoutData = Data()
731743
stdoutPipe.fileHandleForReading.readabilityHandler = { fh in
732-
stdoutData.append(fh.availableData)
744+
dataLock.synchronized {
745+
stdoutData.append(fh.availableData)
746+
}
733747
}
734748

735749
var stderrData = Data()
736750
stderrPipe.fileHandleForReading.readabilityHandler = { fh in
737-
stderrData.append(fh.availableData)
751+
dataLock.synchronized {
752+
stderrData.append(fh.availableData)
753+
}
738754
}
739755

740756
try process.run()
741757
process.waitUntilExit()
742758
stdoutPipe.fileHandleForReading.readabilityHandler = nil
743759
stderrPipe.fileHandleForReading.readabilityHandler = nil
744760

745-
// Drain any data remaining in the pipes
761+
guard process.terminationStatus == 0 else {
762+
throw Error.TerminationStatus(process.terminationStatus)
763+
}
764+
765+
return try dataLock.synchronized {
766+
// Drain any data remaining in the pipes
746767
#if DARWIN_COMPATIBILITY_TESTS
747-
// Use old API for now
748-
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
749-
stderrData.append(stderrPipe.fileHandleForReading.availableData)
768+
// Use old API for now
769+
stdoutData.append(stdoutPipe.fileHandleForReading.availableData)
770+
stderrData.append(stderrPipe.fileHandleForReading.availableData)
750771
#else
751-
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
752-
stdoutData.append(d)
753-
}
772+
if let d = try stdoutPipe.fileHandleForReading.readToEnd() {
773+
stdoutData.append(d)
774+
}
754775

755-
if let d = try stderrPipe.fileHandleForReading.readToEnd() {
756-
stderrData.append(d)
757-
}
776+
if let d = try stderrPipe.fileHandleForReading.readToEnd() {
777+
stderrData.append(d)
778+
}
758779
#endif
759780

760-
guard process.terminationStatus == 0 else {
761-
throw Error.TerminationStatus(process.terminationStatus)
762-
}
781+
guard let stdout = String(data: stdoutData, encoding: .utf8) else {
782+
throw Error.UnicodeDecodingError(stdoutData)
783+
}
763784

764-
guard let stdout = String(data: stdoutData, encoding: .utf8) else {
765-
throw Error.UnicodeDecodingError(stdoutData)
766-
}
785+
guard let stderr = String(data: stderrData, encoding: .utf8) else {
786+
throw Error.UnicodeDecodingError(stderrData)
787+
}
767788

768-
guard let stderr = String(data: stderrData, encoding: .utf8) else {
769-
throw Error.UnicodeDecodingError(stderrData)
789+
return (stdout, stderr)
770790
}
771-
772-
return (stdout, stderr)
773791
}
774792

775793
private func parseEnv(_ env: String) throws -> [String: String] {

TestFoundation/TestURLSessionFTP.swift

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,13 @@ class FTPDataTask : NSObject {
6767
var cancelExpectation: XCTestExpectation?
6868
var responseReceivedExpectation: XCTestExpectation?
6969
var hasTransferCompleted = false
70-
public var error = false
70+
71+
private var errorLock = NSLock()
72+
private var _error = false
73+
public var error: Bool {
74+
get { errorLock.synchronized { _error } }
75+
set { errorLock.synchronized { _error = newValue } }
76+
}
7177

7278
init(with expectation: XCTestExpectation) {
7379
dataTaskExpectation = expectation

TestFoundation/Utilities.swift

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -645,3 +645,10 @@ extension FileHandle: TextOutputStream {
645645
}
646646
}
647647

648+
extension NSLock {
649+
public func synchronized<T>(_ closure: () throws -> T) rethrows -> T {
650+
self.lock()
651+
defer { self.unlock() }
652+
return try closure()
653+
}
654+
}

0 commit comments

Comments
 (0)