Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore parse errors from pinger when its socket is closed #6953

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 21 additions & 21 deletions ios/PacketTunnelCore/Pinger/TunnelPinger.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,24 +14,12 @@ import WireGuardKit

public final class TunnelPinger: PingerProtocol {
private var sequenceNumber: UInt16 = 0
private let stateLock = NSRecursiveLock()
private let stateLock = NSLock()
private let pingReceiveQueue: DispatchQueue
private let replyQueue: DispatchQueue
private var destAddress: IPv4Address?
private var _onReply: ((PingerReply) -> Void)?
public var onReply: ((PingerReply) -> Void)? {
get {
stateLock.withLock {
return _onReply
}
}
set {
stateLock.withLock {
_onReply = newValue
}
}
}

/// Always accessed from the `replyQueue` and is assigned once, on the main thread of the PacketTunnel. It is thread safe.
public var onReply: ((PingerReply) -> Void)?
private var pingProvider: ICMPPingProvider

private let logger: Logger
Expand All @@ -49,18 +37,26 @@ public final class TunnelPinger: PingerProtocol {

public func openSocket(bindTo interfaceName: String?, destAddress: IPv4Address) throws {
try pingProvider.openICMP(address: destAddress)
self.destAddress = destAddress
stateLock.withLock {
self.destAddress = destAddress
}
pingReceiveQueue.async { [weak self] in
while let self {
do {
let seq = try pingProvider.receiveICMP()

replyQueue.async { [weak self] in
self?.onReply?(PingerReply.success(destAddress, UInt16(seq)))
self?.stateLock.withLock {
self?.onReply?(PingerReply.success(destAddress, UInt16(seq)))
}
}
} catch {
replyQueue.async { [weak self] in
self?.onReply?(PingerReply.parseError(error))
self?.stateLock.withLock {
if self?.destAddress != nil {
self?.onReply?(PingerReply.parseError(error))
}
}
}
return
}
Expand All @@ -69,14 +65,18 @@ public final class TunnelPinger: PingerProtocol {
}

public func closeSocket() {
pingProvider.closeICMP()
self.destAddress = nil
stateLock.withLock {
self.destAddress = nil
pingProvider.closeICMP()
}
}

public func send() throws -> PingerSendResult {
let sequenceNumber = nextSequenceNumber()

guard let destAddress else { throw WireGuardAdapterError.invalidState }
stateLock.lock()
defer { stateLock.unlock() }
guard destAddress != nil else { throw WireGuardAdapterError.invalidState }
// NOTE: we cheat here by returning the destination address we were passed, rather than parsing it from the packet on the other side of the FFI boundary.
try pingProvider.sendICMPPing(seqNumber: sequenceNumber)

Expand Down
Loading