fix(apple): queue path updates onto workQueue (#10896)

Path updates are received on a queue which can be (and is typically) on
a different thread from the one the workQueue runs on. Since we are
sharing instance properties across these two threads, we need to make
sure reads and writes to all properties ideally happen on the same
queue.

Moving `Adapter` to an actor class could solve these issues, but that is
a much larger refactor to be done in #10895 and we'd like to ship this
fix in the next release to verify it fixes our issue.
This commit is contained in:
Jamil
2025-11-17 08:09:47 -08:00
committed by GitHub
parent 91962acb83
commit 164f1976c7
2 changed files with 32 additions and 24 deletions

View File

@@ -127,29 +127,33 @@ class Adapter: @unchecked Sendable {
private lazy var pathUpdateHandler: @Sendable (Network.NWPath) -> Void = { [weak self] path in
guard let self else { return }
if path.status == .unsatisfied {
// Check if we need to set reasserting, avoids OS log spam and potentially other side effects
if self.packetTunnelProvider?.reasserting == false {
// Tell the UI we're not connected
self.packetTunnelProvider?.reasserting = true
}
} else {
if self.packetTunnelProvider?.reasserting == true {
self.packetTunnelProvider?.reasserting = false
}
workQueue.async { [weak self] in
guard let self else { return }
if path.connectivityDifferentFrom(path: lastPath) {
// Tell connlib to reset network state and DNS resolvers, but only do so if our connectivity has
// meaningfully changed. On darwin, this is needed to send packets
// out of a different interface even when 0.0.0.0 is used as the source.
// If our primary interface changes, we can be certain the old socket shouldn't be
// used anymore.
self.sendCommand(.reset("primary network path changed"))
if path.status == .unsatisfied {
// Check if we need to set reasserting, avoids OS log spam and potentially other side effects
if self.packetTunnelProvider?.reasserting == false {
// Tell the UI we're not connected
self.packetTunnelProvider?.reasserting = true
}
} else {
if self.packetTunnelProvider?.reasserting == true {
self.packetTunnelProvider?.reasserting = false
}
if path.connectivityDifferentFrom(path: lastPath) {
// Tell connlib to reset network state and DNS resolvers, but only do so if our connectivity has
// meaningfully changed. On darwin, this is needed to send packets
// out of a different interface even when 0.0.0.0 is used as the source.
// If our primary interface changes, we can be certain the old socket shouldn't be
// used anymore.
self.sendCommand(.reset("primary network path changed"))
}
setSystemDefaultResolvers(path)
lastPath = path
}
setSystemDefaultResolvers(path)
lastPath = path
}
}
@@ -189,14 +193,15 @@ class Adapter: @unchecked Sendable {
deinit {
Log.log("Adapter.deinit")
// Cancel network monitor
networkMonitor?.cancel()
networkMonitor = nil
// Cancel all Tasks - this triggers cooperative cancellation
// Event loop checks Task.isCancelled in its polling loop
// Event consumer will exit when eventSender.deinit closes the stream
eventLoopTask?.cancel()
eventConsumerTask?.cancel()
// Cancel network monitor
networkMonitor?.cancel()
}
func start() throws {

View File

@@ -25,6 +25,9 @@ export default function Apple() {
<Entries downloadLinks={downloadLinks} title="macOS / iOS">
{/* When you cut a release, remove any solved issues from the "known issues" lists over in `client-apps`. This must not be done when the issue's PR merges. */}
<Unreleased>
<ChangeItem pull="10986">
Fixes a minor race condition that could arise on sign out.
</ChangeItem>
<ChangeItem pull="10855">
Fixes an issue on macOS where the <code>utun</code> index would
auto-increment by itself on configuration updates.