diff --git a/Sources/SwiftNetwork/Parameters/Parameters.swift b/Sources/SwiftNetwork/Parameters/Parameters.swift index 2c4ce14..427a089 100644 --- a/Sources/SwiftNetwork/Parameters/Parameters.swift +++ b/Sources/SwiftNetwork/Parameters/Parameters.swift @@ -46,7 +46,7 @@ public struct Parameters: Hashable, CustomStringConvertible { } } - public enum MultipathServiceType: UInt8, Hashable, CustomStringConvertible { + public enum MultipathServiceType: UInt8, Hashable, CustomStringConvertible, Sendable { case disabled = 0 case handover = 1 case interactive = 2 @@ -66,7 +66,7 @@ public struct Parameters: Hashable, CustomStringConvertible { } } - public enum ExpiredDNSBehavior: Hashable, CustomStringConvertible { + public enum ExpiredDNSBehavior: Hashable, CustomStringConvertible, Sendable { /// Let the system determine whether or not to allow expired DNS answers case systemDefault /// Explicitly allow the use of expired DNS answers @@ -87,7 +87,7 @@ public struct Parameters: Hashable, CustomStringConvertible { } } - public enum ServiceClass: UInt8, CustomStringConvertible { + public enum ServiceClass: UInt8, CustomStringConvertible, Sendable { /// Default priority traffic case bestEffort = 0 /// Bulk traffic, or traffic that can be de-prioritized behind foreground traffic diff --git a/Sources/SwiftNetwork/Parameters/PathParameters.swift b/Sources/SwiftNetwork/Parameters/PathParameters.swift index b79062f..5144883 100644 --- a/Sources/SwiftNetwork/Parameters/PathParameters.swift +++ b/Sources/SwiftNetwork/Parameters/PathParameters.swift @@ -29,7 +29,7 @@ internal import os @available(Network 0.1.0, *) struct PathParameters: Hashable, CustomStringConvertible { - struct ProcessPathValue: Hashable { + struct ProcessPathValue: Hashable, Sendable { // Parameters that influence path selection for process delegation, by value, that // can be compared and copied. // These items are used for compatibility evaluation for most modes. @@ -70,7 +70,7 @@ struct PathParameters: Hashable, CustomStringConvertible { #endif } - struct PathValue: Hashable { + struct PathValue: Hashable, Sendable { // Parameters that influence path selection, by value, that can be compared and copied // These items are used for compatibility evaluation var trafficClass: UInt32 = 0 @@ -135,7 +135,7 @@ struct PathParameters: Hashable, CustomStringConvertible { } } - struct JoinablePathValue: Hashable { + struct JoinablePathValue: Hashable, Sendable { // Parameters that influence path selection but do not influence compatibility when joining, // by value, that can be compared and copied // These items are not used for compatibility evaluation for joining protocol stacks. @@ -229,12 +229,65 @@ struct PathParameters: Hashable, CustomStringConvertible { func hash(into hasher: inout Hasher) { hasher.combine(storage) } + + init() {} + + init(storage: Storage) { + self.storage = storage + } + + func copy() -> InterfacePreferenceValuesBacking { + .init(storage: storage) + } } - var backing: InterfacePreferenceValuesBacking? + private var backing: InterfacePreferenceValuesBacking? - mutating func setupBacking() { + /// Guarantees a backing that this value uniquely owns, allocating one if absent and copying + /// it first if it is shared. + /// + /// Every mutating path must go through here. Mutating `backing` directly would let a write + /// land on storage another copy can see, which is what the copy-on-write is preventing. + private mutating func ensureUniquelyReferencedBacking() { if self.backing == nil { self.backing = InterfacePreferenceValuesBacking() + } else if !isKnownUniquelyReferenced(&self.backing) { + self.backing = self.backing?.copy() + } + } + + var requiredInterface: Interface? { + get { self.backing?.storage.requiredInterface } + set { + self.ensureUniquelyReferencedBacking() + self.backing!.storage.requiredInterface = newValue + } + } + var prohibitedInterfaceTypes: Deque? { + get { self.backing?.storage.prohibitedInterfaceTypes } + set { + self.ensureUniquelyReferencedBacking() + self.backing!.storage.prohibitedInterfaceTypes = newValue + } + } + var prohibitedInterfaceSubtypes: Deque? { + get { self.backing?.storage.prohibitedInterfaceSubtypes } + set { + self.ensureUniquelyReferencedBacking() + self.backing!.storage.prohibitedInterfaceSubtypes = newValue + } + } + var preferredInterfaceSubtypes: Deque? { + get { self.backing?.storage.preferredInterfaceSubtypes } + set { + self.ensureUniquelyReferencedBacking() + self.backing!.storage.preferredInterfaceSubtypes = newValue + } + } + var prohibitedInterfaces: Deque? { + get { self.backing?.storage.prohibitedInterfaces } + set { + self.ensureUniquelyReferencedBacking() + self.backing!.storage.prohibitedInterfaces = newValue } } @@ -252,38 +305,33 @@ struct PathParameters: Hashable, CustomStringConvertible { var interfacePreferenceValues = InterfacePreferenceValues() var requiredInterface: Interface? { - get { interfacePreferenceValues.backing?.storage.requiredInterface } + get { self.interfacePreferenceValues.requiredInterface } set { - interfacePreferenceValues.setupBacking() - interfacePreferenceValues.backing!.storage.requiredInterface = newValue + self.interfacePreferenceValues.requiredInterface = newValue } } var prohibitedInterfaceTypes: Deque? { - get { interfacePreferenceValues.backing?.storage.prohibitedInterfaceTypes } + get { self.interfacePreferenceValues.prohibitedInterfaceTypes } set { - interfacePreferenceValues.setupBacking() - interfacePreferenceValues.backing!.storage.prohibitedInterfaceTypes = newValue + self.interfacePreferenceValues.prohibitedInterfaceTypes = newValue } } var prohibitedInterfaceSubtypes: Deque? { - get { interfacePreferenceValues.backing?.storage.prohibitedInterfaceSubtypes } + get { self.interfacePreferenceValues.prohibitedInterfaceSubtypes } set { - interfacePreferenceValues.setupBacking() - interfacePreferenceValues.backing!.storage.prohibitedInterfaceSubtypes = newValue + self.interfacePreferenceValues.prohibitedInterfaceSubtypes = newValue } } var preferredInterfaceSubtypes: Deque? { - get { interfacePreferenceValues.backing?.storage.preferredInterfaceSubtypes } + get { self.interfacePreferenceValues.preferredInterfaceSubtypes } set { - interfacePreferenceValues.setupBacking() - interfacePreferenceValues.backing!.storage.preferredInterfaceSubtypes = newValue + self.interfacePreferenceValues.preferredInterfaceSubtypes = newValue } } var prohibitedInterfaces: Deque? { - get { interfacePreferenceValues.backing?.storage.prohibitedInterfaces } + get { self.interfacePreferenceValues.prohibitedInterfaces } set { - interfacePreferenceValues.setupBacking() - interfacePreferenceValues.backing!.storage.prohibitedInterfaces = newValue + self.interfacePreferenceValues.prohibitedInterfaces = newValue } } #if NETWORK_PRIVATE || NETWORK_DRIVERKIT @@ -367,6 +415,10 @@ struct PathParameters: Hashable, CustomStringConvertible { init() {} } +// @unchecked Sendable because access is controlled by getters and copy-on-write setters giving this value semantics. +@available(Network 0.1.0, *) +extension PathParameters.InterfacePreferenceValues: @unchecked Sendable {} + // MARK: - Copying and comparing @available(Network 0.1.0, *) extension PathParameters { diff --git a/Tests/SwiftNetworkTests/SwiftNetworkPathParametersTests.swift b/Tests/SwiftNetworkTests/SwiftNetworkPathParametersTests.swift new file mode 100644 index 0000000..e392ac5 --- /dev/null +++ b/Tests/SwiftNetworkTests/SwiftNetworkPathParametersTests.swift @@ -0,0 +1,75 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift open source project +// +// Copyright (c) 2026 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 +// +// See LICENSE.txt for license information +// See CONTRIBUTORS.txt for the list of Swift project authors +// +// SPDX-License-Identifier: Apache-2.0 +// +//===----------------------------------------------------------------------===// + +import XCTest + +#if canImport(SwiftNetwork) +@_spi(Essentials) @testable import SwiftNetwork +#endif + +@available(Network 0.1.0, *) +final class SwiftNetworkPathParametersTests: NetTestCase { + // `PathParameters` has value semantics, so copies must be independent. Its interface preferences + // live behind a shared class, so without copy-on-write a write through one copy is visible + // through the other — callers would observe changes they never made. + func testWritingInterfacePreferenceDoesNotAffectACopy() { + var original = PathParameters() + original.prohibitedInterfaceTypes = [.wifi] + + var copy = original + copy.prohibitedInterfaceTypes = [.cellular] + + XCTAssertEqual(original.prohibitedInterfaceTypes, [.wifi], "original was mutated through the copy") + XCTAssertEqual(copy.prohibitedInterfaceTypes, [.cellular], "copy did not take the new value") + } + + // The whole backing must be copied, not just the field being assigned; otherwise writing one + // field leaks every other field into the copy that shares the backing. + func testWritingOneFieldDoesNotLeakOthersIntoACopy() { + var original = PathParameters() + original.prohibitedInterfaceTypes = [.wifi] + + var copy = original + copy.preferredInterfaceSubtypes = [.wifiAWDL] + + XCTAssertNil(original.preferredInterfaceSubtypes, "unrelated field leaked into the original") + XCTAssertEqual(original.prohibitedInterfaceTypes, [.wifi], "original lost its own value") + XCTAssertEqual(copy.prohibitedInterfaceTypes, [.wifi], "copy did not inherit the original value") + } + + // Mutating the original after a copy is taken must not write through to the copy either; the + // copy-on-write has to trigger whichever side is written first. + func testWritingTheOriginalDoesNotAffectAnEarlierCopy() { + var original = PathParameters() + original.prohibitedInterfaceTypes = [.wifi] + + let copy = original + original.prohibitedInterfaceTypes = [.cellular] + + XCTAssertEqual(copy.prohibitedInterfaceTypes, [.wifi], "copy was mutated through the original") + XCTAssertEqual(original.prohibitedInterfaceTypes, [.cellular], "original did not take the new value") + } + + // Reading must not allocate a backing. An absent backing and an allocated-but-empty one are not + // equal, so a read that allocates would make two untouched values compare unequal. + func testReadingAPreferenceDoesNotMakeValuesUnequal() { + let read = PathParameters() + let untouched = PathParameters() + + XCTAssertNil(read.prohibitedInterfaceTypes) + XCTAssertNil(read.requiredInterface) + + XCTAssertEqual(read.interfacePreferenceValues, untouched.interfacePreferenceValues) + } +}