From 1577aeb03ef1733090c71fe8f31c94108b9c6e40 Mon Sep 17 00:00:00 2001 From: Honza Dvorsky Date: Tue, 3 Dec 2024 12:25:50 +0100 Subject: [PATCH] Enable strict concurrency --- Package.swift | 6 +++ .../AsyncDNSResolver/c-ares/AresChannel.swift | 19 +++++---- .../AsyncDNSResolver/c-ares/AresOptions.swift | 4 +- .../c-ares/DNSResolver_c-ares.swift | 33 ++++++++------- .../dnssd/DNSResolver_dnssd.swift | 6 +-- .../c-ares/AresChannelTests.swift | 2 +- .../c-ares/CAresDNSResolverTests.swift | 40 ++++++++++--------- .../dnssd/DNSSDDNSResolverTests.swift | 40 ++++++++++--------- 8 files changed, 83 insertions(+), 67 deletions(-) diff --git a/Package.swift b/Package.swift index 6de0490..3ce0fc6 100644 --- a/Package.swift +++ b/Package.swift @@ -50,3 +50,9 @@ let package = Package( ], cLanguageStandard: .gnu11 ) + +for target in package.targets { + var settings = target.swiftSettings ?? [] + settings.append(.enableExperimentalFeature("StrictConcurrency=complete")) + target.swiftSettings = settings +} diff --git a/Sources/AsyncDNSResolver/c-ares/AresChannel.swift b/Sources/AsyncDNSResolver/c-ares/AresChannel.swift index 62c9001..744fbeb 100644 --- a/Sources/AsyncDNSResolver/c-ares/AresChannel.swift +++ b/Sources/AsyncDNSResolver/c-ares/AresChannel.swift @@ -18,17 +18,20 @@ import Foundation // MARK: - ares_channel @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -class AresChannel { - let pointer: UnsafeMutablePointer - let lock = NSLock() +final class AresChannel: @unchecked Sendable { + private let locked_pointer: UnsafeMutablePointer + private let lock = NSLock() - private var underlying: ares_channel? { - self.pointer.pointee + // For testing only. + var underlying: ares_channel? { + self.locked_pointer.pointee } deinit { - ares_destroy(pointer.pointee) - pointer.deallocate() + // Safe to perform without the lock, as in deinit we know that no more + // strong references to self exist, so nobody can be holding the lock. + ares_destroy(locked_pointer.pointee) + locked_pointer.deallocate() ares_library_cleanup() } @@ -49,7 +52,7 @@ class AresChannel { try checkAresResult { ares_set_sortlist(pointer.pointee, sortlist) } } - self.pointer = pointer + self.locked_pointer = pointer } func withChannel(_ body: (ares_channel) -> Void) { diff --git a/Sources/AsyncDNSResolver/c-ares/AresOptions.swift b/Sources/AsyncDNSResolver/c-ares/AresOptions.swift index 2b4c52d..65f5719 100644 --- a/Sources/AsyncDNSResolver/c-ares/AresOptions.swift +++ b/Sources/AsyncDNSResolver/c-ares/AresOptions.swift @@ -19,7 +19,7 @@ import CAsyncDNSResolver @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) extension CAresDNSResolver { /// Options for ``CAresDNSResolver``. - public struct Options { + public struct Options: Sendable { public static var `default`: Options { .init() } @@ -91,7 +91,7 @@ extension CAresDNSResolver { @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) extension CAresDNSResolver.Options { - public struct Flags: OptionSet { + public struct Flags: OptionSet, Sendable { public let rawValue: Int32 public init(rawValue: Int32) { diff --git a/Sources/AsyncDNSResolver/c-ares/DNSResolver_c-ares.swift b/Sources/AsyncDNSResolver/c-ares/DNSResolver_c-ares.swift index ba0864a..3d685c6 100644 --- a/Sources/AsyncDNSResolver/c-ares/DNSResolver_c-ares.swift +++ b/Sources/AsyncDNSResolver/c-ares/DNSResolver_c-ares.swift @@ -13,10 +13,11 @@ //===----------------------------------------------------------------------===// import CAsyncDNSResolver +import Foundation /// ``DNSResolver`` implementation backed by c-ares C library. @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -public class CAresDNSResolver: DNSResolver { +public final class CAresDNSResolver: DNSResolver, Sendable { let options: Options let ares: Ares @@ -121,18 +122,15 @@ extension QueryType { // MARK: - c-ares query wrapper @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -class Ares { +final class Ares: Sendable { typealias QueryCallback = @convention(c) ( UnsafeMutableRawPointer?, CInt, CInt, UnsafeMutablePointer?, CInt ) -> Void - let options: AresOptions - let channel: AresChannel - + private let channel: AresChannel private let queryProcessor: QueryProcessor init(options: AresOptions) throws { - self.options = options self.channel = try AresChannel(options: options) // Need to call `ares_process` or `ares_process_fd` for query callbacks to happen @@ -145,7 +143,8 @@ class Ares { name: String, replyParser: ReplyParser ) async throws -> ReplyParser.Reply { - try await withTaskCancellationHandler( + let channel = self.channel + return try await withTaskCancellationHandler( operation: { try await withCheckedThrowingContinuation { continuation in let handler = QueryReplyHandler(parser: replyParser, continuation) @@ -178,7 +177,7 @@ class Ares { } }, onCancel: { - self.channel.withChannel { channel in + channel.withChannel { channel in ares_cancel(channel) } } @@ -198,16 +197,18 @@ extension Ares { // https://github.com/dimbleby/c-ares-resolver/blob/master/src/unix/eventloop.rs // ignore-unacceptable-language // https://github.com/dimbleby/rust-c-ares/blob/master/src/channel.rs // ignore-unacceptable-language // https://github.com/dimbleby/rust-c-ares/blob/master/examples/event-loop.rs // ignore-unacceptable-language - class QueryProcessor { + final class QueryProcessor: @unchecked Sendable { static let defaultPollInterval: UInt64 = 10 * 1_000_000 // 10ms private let channel: AresChannel private let pollIntervalNanos: UInt64 - private var pollingTask: Task? + private let lock = NSLock() + private var locked_pollingTask: Task? deinit { - self.pollingTask?.cancel() + // No need to lock here as there can exist no more strong references to self. + self.locked_pollingTask?.cancel() } init(channel: AresChannel, pollIntervalNanos: UInt64 = QueryProcessor.defaultPollInterval) { @@ -218,7 +219,7 @@ extension Ares { /// Asks c-ares for the set of socket descriptors we are waiting on for the `ares_channel`'s pending queries /// then call `ares_process_fd` if any is ready for read and/or write. /// c-ares returns up to `ARES_GETSOCK_MAXNUM` socket descriptors only. If more are in use (unlikely) they are not reported back. - func poll() async { + func poll() { var socks = [ares_socket_t](repeating: ares_socket_t(), count: Int(ARES_GETSOCK_MAXNUM)) self.channel.withChannel { channel in @@ -249,12 +250,14 @@ extension Ares { } private func schedule() { - self.pollingTask = Task { [weak self] in + self.lock.lock() + defer { self.lock.unlock() } + self.locked_pollingTask = Task { [weak self] in guard let s = self else { return } try await Task.sleep(nanoseconds: s.pollIntervalNanos) - await s.poll() + s.poll() } } } @@ -291,7 +294,7 @@ extension Ares { // MARK: - c-ares query reply parsers protocol AresQueryReplyParser { - associatedtype Reply + associatedtype Reply: Sendable func parse(buffer: UnsafeMutablePointer?, length: CInt) throws -> Reply } diff --git a/Sources/AsyncDNSResolver/dnssd/DNSResolver_dnssd.swift b/Sources/AsyncDNSResolver/dnssd/DNSResolver_dnssd.swift index 7594b98..7f61adc 100644 --- a/Sources/AsyncDNSResolver/dnssd/DNSResolver_dnssd.swift +++ b/Sources/AsyncDNSResolver/dnssd/DNSResolver_dnssd.swift @@ -17,7 +17,7 @@ import dnssd /// ``DNSResolver`` implementation backed by dnssd framework. @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -public struct DNSSDDNSResolver: DNSResolver { +public struct DNSSDDNSResolver: DNSResolver, Sendable { let dnssd: DNSSD init() { @@ -100,7 +100,7 @@ extension QueryType { // MARK: - dnssd query wrapper @available(macOS 10.15, iOS 13, tvOS 13, watchOS 6, *) -struct DNSSD { +struct DNSSD: Sendable { // Reference: https://gist.github.com/fikeminkel/a9c4bc4d0348527e8df3690e242038d3 func query( type: QueryType, @@ -225,7 +225,7 @@ extension DNSSD { // MARK: - dnssd query reply handlers protocol DNSSDQueryReplyHandler { - associatedtype Record + associatedtype Record: Sendable associatedtype Reply func parseRecord(data: UnsafeRawPointer?, length: UInt16) throws -> Record? diff --git a/Tests/AsyncDNSResolverTests/c-ares/AresChannelTests.swift b/Tests/AsyncDNSResolverTests/c-ares/AresChannelTests.swift index cf8340b..29ea213 100644 --- a/Tests/AsyncDNSResolverTests/c-ares/AresChannelTests.swift +++ b/Tests/AsyncDNSResolverTests/c-ares/AresChannelTests.swift @@ -29,7 +29,7 @@ final class AresChannelTests: XCTestCase { guard let channel = try? AresChannel(options: options) else { return XCTFail("Channel not initialized") } - guard let _ = channel.pointer.pointee else { + guard let _ = channel.underlying else { return XCTFail("Underlying ares_channel is nil") } } diff --git a/Tests/AsyncDNSResolverTests/c-ares/CAresDNSResolverTests.swift b/Tests/AsyncDNSResolverTests/c-ares/CAresDNSResolverTests.swift index c48048f..c64452c 100644 --- a/Tests/AsyncDNSResolverTests/c-ares/CAresDNSResolverTests.swift +++ b/Tests/AsyncDNSResolverTests/c-ares/CAresDNSResolverTests.swift @@ -124,7 +124,7 @@ final class CAresDNSResolverTests: XCTestCase { func test_concurrency() async throws { func run( times: Int = 100, - _ query: @escaping (_ index: Int) async throws -> Void + _ query: @Sendable @escaping (_ index: Int) async throws -> Void ) async throws { try await withThrowingTaskGroup(of: Void.self) { group in for i in 1...times { @@ -136,65 +136,67 @@ final class CAresDNSResolverTests: XCTestCase { } } + let resolver = self.resolver! + let verbose = self.verbose try await run { i in - let reply = try await self.resolver.queryA(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryA(name: "apple.com") + if verbose { print("[A] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryAAAA(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryAAAA(name: "apple.com") + if verbose { print("[AAAA] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryNS(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryNS(name: "apple.com") + if verbose { print("[NS] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryCNAME(name: "www.apple.com") - if self.verbose { + let reply = try await resolver.queryCNAME(name: "www.apple.com") + if verbose { print("[CNAME] run #\(i) result: \(String(describing: reply))") } } try await run { i in - let reply = try await self.resolver.querySOA(name: "apple.com") - if self.verbose { + let reply = try await resolver.querySOA(name: "apple.com") + if verbose { print("[SOA] run #\(i) result: \(String(describing: reply))") } } try await run { i in - let reply = try await self.resolver.queryPTR(name: "47.224.172.17.in-addr.arpa") - if self.verbose { + let reply = try await resolver.queryPTR(name: "47.224.172.17.in-addr.arpa") + if verbose { print("[PTR] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryMX(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryMX(name: "apple.com") + if verbose { print("[MX] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryTXT(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryTXT(name: "apple.com") + if verbose { print("[TXT] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.querySRV(name: "_caldavs._tcp.google.com") - if self.verbose { + let reply = try await resolver.querySRV(name: "_caldavs._tcp.google.com") + if verbose { print("[SRV] run #\(i) result: \(reply)") } } diff --git a/Tests/AsyncDNSResolverTests/dnssd/DNSSDDNSResolverTests.swift b/Tests/AsyncDNSResolverTests/dnssd/DNSSDDNSResolverTests.swift index 9f4092b..f7bba8c 100644 --- a/Tests/AsyncDNSResolverTests/dnssd/DNSSDDNSResolverTests.swift +++ b/Tests/AsyncDNSResolverTests/dnssd/DNSSDDNSResolverTests.swift @@ -143,7 +143,7 @@ final class DNSSDDNSResolverTests: XCTestCase { func test_concurrency() async throws { func run( times: Int = 100, - _ query: @escaping (_ index: Int) async throws -> Void + _ query: @Sendable @escaping (_ index: Int) async throws -> Void ) async throws { try await withThrowingTaskGroup(of: Void.self) { group in for i in 1...times { @@ -155,65 +155,67 @@ final class DNSSDDNSResolverTests: XCTestCase { } } + let resolver = self.resolver! + let verbose = self.verbose try await run { i in - let reply = try await self.resolver.queryA(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryA(name: "apple.com") + if verbose { print("[A] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryAAAA(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryAAAA(name: "apple.com") + if verbose { print("[AAAA] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryNS(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryNS(name: "apple.com") + if verbose { print("[NS] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryCNAME(name: "www.apple.com") - if self.verbose { + let reply = try await resolver.queryCNAME(name: "www.apple.com") + if verbose { print("[CNAME] run #\(i) result: \(String(describing: reply))") } } try await run { i in - let reply = try await self.resolver.querySOA(name: "apple.com") - if self.verbose { + let reply = try await resolver.querySOA(name: "apple.com") + if verbose { print("[SOA] run #\(i) result: \(String(describing: reply))") } } try await run { i in - let reply = try await self.resolver.queryPTR(name: "47.224.172.17.in-addr.arpa") - if self.verbose { + let reply = try await resolver.queryPTR(name: "47.224.172.17.in-addr.arpa") + if verbose { print("[PTR] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryMX(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryMX(name: "apple.com") + if verbose { print("[MX] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.queryTXT(name: "apple.com") - if self.verbose { + let reply = try await resolver.queryTXT(name: "apple.com") + if verbose { print("[TXT] run #\(i) result: \(reply)") } } try await run { i in - let reply = try await self.resolver.querySRV(name: "_caldavs._tcp.google.com") - if self.verbose { + let reply = try await resolver.querySRV(name: "_caldavs._tcp.google.com") + if verbose { print("[SRV] run #\(i) result: \(reply)") } }