Skip to content

Commit 60efe5b

Browse files
committed
Tear down old path after migration
1 parent 76d7587 commit 60efe5b

4 files changed

Lines changed: 164 additions & 10 deletions

File tree

Sources/SwiftNetwork/QUIC/Migration.swift

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ struct Migration: ~Copyable {
106106
return
107107
}
108108

109+
let oldPath = connection.currentPath
109110
connection.log.notice("Migrating to path \(path.identifier)")
110111
connection.currentPath = path
111112
path.spinValue = connection.initialSpinValue
@@ -128,6 +129,10 @@ struct Migration: ~Copyable {
128129
}
129130
// TODO: Handle preferred address migration
130131

132+
// Remove the path we just migrated away from.
133+
if let oldPath, oldPath != path {
134+
connection.tearDownMigratedPath(oldPath)
135+
}
131136
}
132137

133138
func probingPathCount(_ connection: QUICConnection) -> Int {
@@ -197,16 +202,7 @@ extension QUICConnection {
197202
}
198203
break
199204
case .unavailable:
200-
if path.isOpenForSending, let dcid = path.dcid,
201-
let sequence = remoteCIDs.retire(connectionID: dcid)
202-
{
203-
withPendingItems(
204-
for: .applicationData,
205-
block: {
206-
$0.addRetireConnectionID(FrameRetireConnectionID(sequence: sequence))
207-
}
208-
)
209-
}
205+
retireOutboundCID(forPathGoingAway: path)
210206
path.changeState(to: .routeUnavailable)
211207
break
212208
}
@@ -225,5 +221,35 @@ extension QUICConnection {
225221
sendFrames(on: path)
226222
}
227223
}
224+
225+
// Retires a path's outbound CID and queues a RETIRE_CONNECTION_ID frame for it.
226+
func retireOutboundCID(forPathGoingAway path: QUICPath) {
227+
guard path.isOpenForSending, !path.hasPreAssignedCIDs, let dcid = path.dcid,
228+
let sequence = remoteCIDs.retire(connectionID: dcid)
229+
else {
230+
return
231+
}
232+
withPendingItems(for: .applicationData) {
233+
$0.addRetireConnectionID(FrameRetireConnectionID(sequence: sequence))
234+
}
235+
}
236+
237+
// Removes a path we migrated away from.
238+
func tearDownMigratedPath(_ oldPath: QUICPath) {
239+
guard oldPath !== currentPath else {
240+
log.fault("Refusing to tear down the current path \(oldPath.identifier)")
241+
return
242+
}
243+
log.notice("Tearing down old path \(oldPath.identifier) after migration")
244+
245+
retireOutboundCID(forPathGoingAway: oldPath)
246+
247+
if oldPath.state.isValidStateChange(to: .routeUnavailable) {
248+
oldPath.changeState(to: .routeUnavailable)
249+
}
250+
oldPath.tearDownLowerStack()
251+
multiplexingPaths.removeValue(forKey: oldPath.identifier)
252+
sendFrames()
253+
}
228254
}
229255
#endif

Sources/SwiftNetwork/QUIC/QUICPath.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,10 @@ public final class QUICPath: MultiplexingDatagramPath<QUICConnection>, Equatable
623623
parentProtocol.migration.migrate(to: self, connection: parentProtocol)
624624
}
625625
}
626+
627+
func tearDownLowerStack() {
628+
try? lower.invokeDetach(self.reference)
629+
}
626630
}
627631

628632
// Congestion Control access
Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift open source project
4+
//
5+
// Copyright (c) 2026 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0
7+
//
8+
// See LICENSE.txt for license information
9+
// See CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
// SPDX-License-Identifier: Apache-2.0
12+
//
13+
//===----------------------------------------------------------------------===//
14+
15+
#if !NETWORK_NO_SWIFT_QUIC
16+
17+
import XCTest
18+
19+
#if canImport(SwiftNetwork)
20+
@_spi(Essentials) @_spi(ProtocolProvider) @testable import SwiftNetwork
21+
#elseif canImport(Network)
22+
@_spi(Essentials) @_spi(ProtocolProvider) @testable import Network
23+
#endif
24+
25+
@available(Network 0.1.0, *)
26+
let migrationTestsLogPrefixer: LogPrefixer = LogPrefixer("[MigrationTests]")
27+
28+
@available(Network 0.1.0, *)
29+
final class MigrationTests: XCTestCase {
30+
var connection = QUICConnection(context: .implicitContext)
31+
32+
static let oldCID = QUICConnectionID([0xA1, 0xA2, 0xA3, 0xA4])!
33+
static let newCID = QUICConnectionID([0xB1, 0xB2, 0xB3, 0xB4])!
34+
35+
override func setUp() {
36+
let expectation = XCTestExpectation()
37+
connection.context.async {
38+
try? self.connection.setup(remote: nil, local: nil, parameters: nil, path: nil)
39+
self.connection.recovery = Recovery(logPrefixer: migrationTestsLogPrefixer)
40+
self.connection.recovery.connection = self.connection
41+
expectation.fulfill()
42+
}
43+
wait(for: [expectation], timeout: 5.0)
44+
}
45+
46+
override func tearDown() {
47+
self.connection.currentPath = nil
48+
}
49+
50+
// Builds a path that is open for sending, backed by a lower harness, with its DCID
51+
// registered in `remoteCIDs` so it can be retired. `validated` drives it to the
52+
// validated state so `migrate(to:)` will accept it.
53+
private func makePath(dcid: QUICConnectionID, sequenceNumber: UInt64, validated: Bool) -> QUICPath {
54+
let lower = DatagramLowerHarness(identifier: "\(sequenceNumber)", context: .implicitContext)
55+
lower.connect()
56+
var path = QUICPath(parent: connection)
57+
path.set(interface: nil, priority: 1, isInitial: true) // -> .routeEstablished
58+
path.assignDCID(dcid) // -> .cidAssigned (open for sending)
59+
if validated {
60+
path.changeState(to: .probing)
61+
path.changeState(to: .validated)
62+
}
63+
try? path.attachLowerProtocol(lower.reference, remote: nil, local: nil, parameters: nil, path: nil)
64+
try? connection.remoteCIDs.insert(
65+
sequenceNumber: sequenceNumber,
66+
connectionID: dcid,
67+
token: QUICStatelessResetToken(Array(repeating: UInt8(sequenceNumber & 0xff), count: 16))!
68+
)
69+
return path
70+
}
71+
72+
func testMigrationRemovesOldPathAndRetiresItsCID() {
73+
let expectation = XCTestExpectation()
74+
connection.context.async {
75+
let oldPath = self.makePath(dcid: Self.oldCID, sequenceNumber: 1, validated: false)
76+
let newPath = self.makePath(dcid: Self.newCID, sequenceNumber: 2, validated: true)
77+
78+
self.connection.currentPath = oldPath
79+
self.connection.multiplexingPaths[oldPath.identifier] = oldPath
80+
self.connection.multiplexingPaths[newPath.identifier] = newPath
81+
let oldPathID = oldPath.identifier
82+
83+
self.connection.migration.migrate(to: newPath, connection: self.connection)
84+
85+
// The path we migrated away from is dropped from the connection and its
86+
// remote CID is retired.
87+
XCTAssertNil(self.connection.multiplexingPaths[oldPathID], "old path should be removed from multiplexingPaths")
88+
XCTAssertNil(
89+
self.connection.remoteCIDs.retire(connectionID: Self.oldCID),
90+
"old path's remote CID should be retired by migration"
91+
)
92+
93+
// Only the new path remains, and it is now the current path.
94+
XCTAssertEqual(self.connection.multiplexingPaths.count, 1, "old path should not linger in multiplexingPaths")
95+
XCTAssertEqual(self.connection.currentPath?.identifier, newPath.identifier, "current path should be the new path")
96+
97+
expectation.fulfill()
98+
}
99+
wait(for: [expectation], timeout: 5.0)
100+
}
101+
}
102+
103+
#endif

Tests/SwiftNetworkTests/SwiftNetworkQUICStackTests.swift

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,16 @@ final class SwiftNetworkQUICStackTests: NetTestCase {
452452
}
453453
}
454454

455+
// Currently we only keep the current path object and remove all the
456+
// other path objects we are migrating away from, so the number of
457+
// multiplexing paths should be one.
458+
if case .quic(let clientConnection) = clientQUICReference.reference {
459+
XCTAssertEqual(
460+
clientConnection.multiplexingPaths.count, 1,
461+
"Migrated-away path leaked into multiplexingPaths after migrating to path \(pathIndex)"
462+
)
463+
}
464+
455465
if let dataToSend {
456466
Logger.test.info("Writing data to send on path \(pathIndex)")
457467
_ = clientUpperHarness?.write(dataToSend)
@@ -538,6 +548,17 @@ final class SwiftNetworkQUICStackTests: NetTestCase {
538548
dataToSend: Array("Hello World!".utf8)
539549
)
540550
}
551+
552+
func testQUICStackMigrationDoesNotLeakPaths() {
553+
let ipv4Client = Endpoint(address: IPv4Address(SwiftNetworkQUICStackTests.localIPv4Address)!, port: 1234)
554+
let ipv4Server = Endpoint(address: IPv4Address(SwiftNetworkQUICStackTests.localIPv4Address)!, port: 8080)
555+
quicStackHandshake(
556+
clientEndpoint: ipv4Client,
557+
serverEndpoint: ipv4Server,
558+
migrateCount: 4,
559+
dataToSend: Array("Hello World!".utf8)
560+
)
561+
}
541562
}
542563
#endif
543564
#endif

0 commit comments

Comments
 (0)