From 1ce651040f0220ba2035bc03505ad295e68a56aa Mon Sep 17 00:00:00 2001 From: Aleksandr Shabelnikov Date: Mon, 20 Jul 2026 09:08:01 +0200 Subject: [PATCH] refactor(tunnel): remove memory-triggered reconnect, gate legacy diagnostics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-1 (Measurement Safety) — stop legacy telemetry from contaminating native memory baselines before PR0/PR1 optimization work. Memory pressure: - Remove triggerMemoryPressureReconnect(): the emergency threshold (42 MiB) no longer destroys the WebSocket bridge and schedules a full reconnect. Recreating TLS, queues and buffers near the memory ceiling increased peak usage and obscured the actual growth source. - evaluateMemoryPressure() is now telemetry-only: both warning and emergency emit a throttled os_log entry. No bridge action, no JSONL event recording, no heartbeat rewrite originates from this method. - Remove memoryEmergencyReconnectInProgress state and all references. Measurement configuration: - Add a real Measurement build configuration (release-based) to project.yml with FPTN_MEASUREMENT_BUILD compilation condition. - recordProviderEvent() uses @autoclosure + #if gate: interpolated message strings are never constructed in Measurement builds. - updateDiagnosticsHeartbeat() returns before constructing the full runtime snapshot (native status query, ISO-8601 dates, string copies) in Measurement builds. - emitTelemetry() samples only Mach memory counters in Measurement builds, skipping the full TunnelRuntimeSnapshot construction. - TunnelLogLevelStore forces .warning in Measurement builds so info/debug messages never reach the formatter or redactor. - RingLogSink.tunnel.write() gated behind #if DEBUG: the 1 MiB ring buffer and 512 KiB file sink are compiled out of Release builds. Release builds do not retain provider text in the application-owned ring or shared log file; sparse messages continue through os_log. - TunnelDiagnosticsStore.providerRecordingEnabled runtime guard remains as defense-in-depth for non-Measurement builds. Thresholds (30/42 MiB) retained as telemetry-only markers. Recalibration deferred until real-device measurements post-PR0. Tests: 22/22 passed. Debug + Measurement configurations build. --- FptnVPNTests/FptnVPNTests.swift | 30 +++++ FptnVPNTunnel/Logging/TunnelLogger.swift | 26 ++++- FptnVPNTunnel/PacketTunnelProvider.swift | 103 +++++++++--------- .../TunnelDiagnosticsStore.swift | 31 ++++++ project.yml | 8 ++ 5 files changed, 142 insertions(+), 56 deletions(-) diff --git a/FptnVPNTests/FptnVPNTests.swift b/FptnVPNTests/FptnVPNTests.swift index 7ef3967..f24c307 100644 --- a/FptnVPNTests/FptnVPNTests.swift +++ b/FptnVPNTests/FptnVPNTests.swift @@ -226,6 +226,36 @@ struct FptnVPNTests { #expect(rssWarningWithoutFootprint.level == .warning) } + // PR-1 (Measurement Safety): the emergency threshold (42 MiB) must not + // stop, replace, or reconnect the native WebSocket bridge. The provider's + // response at both warning and emergency is log-only — no bridge + // destruction, generation bump, reconnect scheduling, or state transition. + @Test func memoryPressureActionIsLogOnlyAtAllThresholds() { + #expect(TunnelMemoryPressureAction.action(for: .normal) == .none) + #expect(TunnelMemoryPressureAction.action(for: .warning) == .logOnly) + #expect(TunnelMemoryPressureAction.action(for: .emergency) == .logOnly) + + let emergency = TunnelMemoryPressureSnapshot( + residentBytes: 50 * 1024 * 1024, + physFootprintBytes: 45 * 1024 * 1024 + ) + #expect(emergency.level == .emergency) + #expect(TunnelMemoryPressureAction.action(for: emergency.level) == .logOnly) + + let exactlyAtEmergency = TunnelMemoryPressureSnapshot( + residentBytes: nil, + physFootprintBytes: TunnelMemoryPressureSnapshot.emergencyThresholdBytes + ) + #expect(exactlyAtEmergency.level == .emergency) + + let oneBelowEmergency = TunnelMemoryPressureSnapshot( + residentBytes: nil, + physFootprintBytes: TunnelMemoryPressureSnapshot.emergencyThresholdBytes - 1 + ) + #expect(oneBelowEmergency.level == .warning) + #expect(TunnelMemoryPressureAction.action(for: oneBelowEmergency.level) == .logOnly) + } + @Test func diagnosticsStoreReadsOldHeartbeatWithoutFootprint() throws { let root = try temporaryDiagnosticsDirectory() try """ diff --git a/FptnVPNTunnel/Logging/TunnelLogger.swift b/FptnVPNTunnel/Logging/TunnelLogger.swift index 718ff96..9a80003 100644 --- a/FptnVPNTunnel/Logging/TunnelLogger.swift +++ b/FptnVPNTunnel/Logging/TunnelLogger.swift @@ -53,7 +53,16 @@ private final class TunnelLogLevelStore: @unchecked Sendable { static let shared = TunnelLogLevelStore() private let lock = NSLock() - private var current: TunnelRuntimeLogLevel = .info + // PR-1 (Measurement Safety): Measurement builds start at .warning so + // early startup messages (before setTunnelLogLevel is called) never + // reach the formatter, redactor, or os_log at info/debug level. + private var current: TunnelRuntimeLogLevel = { + #if FPTN_MEASUREMENT_BUILD + return .warning + #else + return .info + #endif + }() func get() -> TunnelRuntimeLogLevel { lock.lock() @@ -68,7 +77,14 @@ private final class TunnelLogLevelStore: @unchecked Sendable { func set(_ level: TunnelRuntimeLogLevel) { lock.lock() + // PR-1 (Measurement Safety): Measurement builds are locked to + // warning-only so info/debug messages never reach the formatter, + // redactor, or os_log during memory profiling. + #if FPTN_MEASUREMENT_BUILD + current = .warning + #else current = level + #endif lock.unlock() } } @@ -141,8 +157,14 @@ struct TunnelLogHandler: Logging.LogHandler { } os_log("%{public}@", log: osLog, type: osType, text) - // 2. Ring buffer → background flush to shared file + // PR-1 (Measurement Safety): the 1 MiB in-memory ring buffer and its + // 512 KiB file sink are compiled out of Release builds. Release + // builds do not retain provider text in the application-owned ring + // or shared log file; sparse messages continue through os_log. + // Debug/internal builds retain the ring for developer diagnostics. + #if DEBUG RingLogSink.tunnel.write(text) + #endif } private func format( diff --git a/FptnVPNTunnel/PacketTunnelProvider.swift b/FptnVPNTunnel/PacketTunnelProvider.swift index 1b3aada..ae80b55 100644 --- a/FptnVPNTunnel/PacketTunnelProvider.swift +++ b/FptnVPNTunnel/PacketTunnelProvider.swift @@ -202,7 +202,6 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { private var lastStopReasonRawValue: Int? private var counters = PacketCounters() private var lastMemoryWarningAt: Date? - private var memoryEmergencyReconnectInProgress = false private var readBackpressureUntil: Date? private var readBackpressureWorkItem: DispatchWorkItem? private var consecutiveSendFailureBatches = 0 @@ -267,7 +266,6 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { lastNetworkPathSummary = "unknown" counters = PacketCounters() lastMemoryWarningAt = nil - memoryEmergencyReconnectInProgress = false readBackpressureUntil = nil readBackpressureWorkItem = nil consecutiveSendFailureBatches = 0 @@ -540,7 +538,6 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { self.isReadLoopActive = true self.reconnectAttempt = 0 self.lastTransportError = nil - self.memoryEmergencyReconnectInProgress = false self.stateLock.unlock() self.updateRuntimeState(.connected, reason: "websocket connected and settings applied") @@ -557,7 +554,6 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { stateLock.lock() reconnectAttempt = 0 lastTransportError = nil - memoryEmergencyReconnectInProgress = false stateLock.unlock() updateRuntimeState(.connected, reason: "transport recovered") @@ -1392,13 +1388,25 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { } private func emitTelemetry() { + #if FPTN_MEASUREMENT_BUILD + // PR-1 (Measurement Safety): sample only Mach memory counters. + // Avoids constructing the full TunnelRuntimeSnapshot (native + // status query, ISO-8601 dates, string copies, packet counters) + // on every 15-second tick during memory profiling. + let memory = TunnelMemoryPressureSnapshot( + residentBytes: residentMemoryBytes(), + physFootprintBytes: physFootprintBytes() + ) + evaluateMemoryPressure(memory: memory) + #else let snapshot = currentSnapshot() let memory = TunnelMemoryPressureSnapshot( residentBytes: snapshot.memoryResidentBytes, physFootprintBytes: snapshot.memoryPhysFootprintBytes ) updateDiagnosticsHeartbeat(snapshot: snapshot, lastEvent: "telemetry") - evaluateMemoryPressure(snapshot: snapshot, memory: memory) + evaluateMemoryPressure(memory: memory) + #endif } // MARK: - State helpers @@ -1486,29 +1494,49 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { ) } + // PR-1 (Measurement Safety): @autoclosure prevents the interpolated + // message string from being constructed in Measurement builds. + // The #if gate eliminates the entire call (including category string + // literals) at compile time rather than relying on a runtime guard + // inside TunnelDiagnosticsStore. private func recordProviderEvent( category: String, - message: String, + message: @autoclosure () -> String, runtimeState: String? = nil, generation: Int? = nil, reconnectAttempt: Int? = nil, pathSatisfied: Bool? = nil ) { + #if FPTN_MEASUREMENT_BUILD + return + #else TunnelDiagnosticsStore.shared.recordProviderEvent( category: category, - message: message, + message: message(), runtimeState: runtimeState, generation: generation, reconnectAttempt: reconnectAttempt, pathSatisfied: pathSatisfied ) + #endif } - private func updateDiagnosticsHeartbeat(lastEvent: String) { - updateDiagnosticsHeartbeat(snapshot: currentSnapshot(), lastEvent: lastEvent) + // PR-1 (Measurement Safety): @autoclosure prevents interpolated + // lastEvent strings (e.g. "state=\(state.rawValue)") from being + // constructed in Measurement builds. + private func updateDiagnosticsHeartbeat(lastEvent: @autoclosure () -> String) { + #if FPTN_MEASUREMENT_BUILD + return + #else + updateDiagnosticsHeartbeat(snapshot: currentSnapshot(), lastEvent: lastEvent()) + #endif } - private func updateDiagnosticsHeartbeat(snapshot: TunnelRuntimeSnapshot, lastEvent: String) { + private func updateDiagnosticsHeartbeat(snapshot: TunnelRuntimeSnapshot, lastEvent: @autoclosure () -> String) { + #if FPTN_MEASUREMENT_BUILD + return + #endif + let event = lastEvent() let generation: Int let pathSatisfied: Bool stateLock.lock() @@ -1529,7 +1557,7 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { websocketRunning: snapshot.websocketRunning, lastTransportError: snapshot.lastTransportError ?? snapshot.websocketLastError, lastStopReason: snapshot.lastStopReason, - lastEvent: lastEvent, + lastEvent: event, lastInboundActivityAt: snapshot.lastInboundActivityAt, lastOutboundActivityAt: snapshot.lastOutboundActivityAt, packetFlowReadPackets: snapshot.packetFlowReadPackets, @@ -1580,14 +1608,21 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { return UInt64(info.phys_footprint) } + // PR-1 (Measurement Safety): memory pressure is now telemetry-only. + // The previous implementation destroyed the native WebSocket bridge and + // scheduled a full reconnect at the emergency threshold (42 MiB). + // Recreating TLS, WebSocket, queues and buffers near the memory ceiling + // increased peak usage and obscured the original source of memory growth. + // Both warning and emergency levels now emit a throttled OSLog entry only. + // No bridge stop, bridge creation, reconnect scheduling, JSONL event + // recording, or heartbeat rewrite originates from this method. private func evaluateMemoryPressure( - snapshot: TunnelRuntimeSnapshot, memory: TunnelMemoryPressureSnapshot ) { switch memory.level { case .normal: return - case .warning: + case .warning, .emergency: let now = Date() var shouldLog = false stateLock.lock() @@ -1598,47 +1633,7 @@ final class PacketTunnelProvider: NEPacketTunnelProvider, @unchecked Sendable { } stateLock.unlock() guard shouldLog else { return } - logger.warning("Tunnel memory pressure warning \(memory.description)") - recordProviderEvent(category: "memory", message: "memory_pressure_warning \(memory.description)", runtimeState: snapshot.runtimeState.rawValue) - updateDiagnosticsHeartbeat(snapshot: snapshot, lastEvent: "memory_pressure_warning") - case .emergency: - triggerMemoryPressureReconnect(snapshot: snapshot, memory: memory) - } - } - - private func triggerMemoryPressureReconnect( - snapshot: TunnelRuntimeSnapshot, - memory: TunnelMemoryPressureSnapshot - ) { - stateLock.lock() - guard !memoryEmergencyReconnectInProgress, - !shutdownRequested, - runtimeState == .connected || runtimeState == .reasserting || runtimeState == .waitingForNetwork else { - stateLock.unlock() - return - } - memoryEmergencyReconnectInProgress = true - websocketGeneration += 1 - lastTransportError = "provider_memory_pressure" - stateLock.unlock() - - logger.warning("Tunnel memory pressure emergency, reconnecting websocket \(memory.description)") - recordProviderEvent( - category: "memory", - message: "memory_pressure_emergency reconnect \(memory.description)", - runtimeState: snapshot.runtimeState.rawValue - ) - updateDiagnosticsHeartbeat(snapshot: snapshot, lastEvent: "memory_pressure_emergency") - cancelPendingReconnect() - replaceWebSocketClient(with: nil, stopCurrent: true) - - switch scheduleReconnectIfPossible() { - case .scheduled: - updateRuntimeState(.reasserting, reason: "provider_memory_pressure") - case .waitingForNetwork: - updateRuntimeState(.waitingForNetwork, reason: "provider_memory_pressure") - case .unavailable: - failRuntimeTunnel(reason: "provider_memory_pressure") + logger.warning("Tunnel memory pressure \(memory.level.rawValue) \(memory.description)") } } diff --git a/SharedTunnelRuntime/TunnelDiagnosticsStore.swift b/SharedTunnelRuntime/TunnelDiagnosticsStore.swift index 8c66304..7b6ddf4 100644 --- a/SharedTunnelRuntime/TunnelDiagnosticsStore.swift +++ b/SharedTunnelRuntime/TunnelDiagnosticsStore.swift @@ -41,6 +41,23 @@ enum TunnelMemoryPressureLevel: String, Codable, Sendable { case emergency } +// PR-1 (Measurement Safety): the provider's response to memory pressure. +// Both warning and emergency are log-only — no bridge stop, replacement, +// or reconnect is ever triggered by memory thresholds. +enum TunnelMemoryPressureAction: Equatable, Sendable { + case none + case logOnly + + static func action(for level: TunnelMemoryPressureLevel) -> TunnelMemoryPressureAction { + switch level { + case .normal: + return .none + case .warning, .emergency: + return .logOnly + } + } +} + struct TunnelMemoryPressureSnapshot: Equatable, Sendable { static let warningThresholdBytes: UInt64 = 30 * 1024 * 1024 static let emergencyThresholdBytes: UInt64 = 42 * 1024 * 1024 @@ -141,6 +158,18 @@ struct TunnelProviderFailureReport: Equatable, Sendable { final class TunnelDiagnosticsStore: @unchecked Sendable { static let shared = TunnelDiagnosticsStore() + // PR-1 (Measurement Safety): compile-time gate. In Measurement builds, + // recordProviderEvent and writeHeartbeat are no-ops. This is a computed + // property (not mutable state) so no cross-thread race is possible and + // no call can accidentally write diagnostics before startTunnel runs. + static var providerRecordingEnabled: Bool { + #if FPTN_MEASUREMENT_BUILD + return false + #else + return true + #endif + } + private static let appGroup = "group.net.mrmidi.FptnVPN" private let queue = DispatchQueue(label: "org.fptn.diagnostics-store", qos: .utility) private let encoder = JSONEncoder() @@ -170,6 +199,7 @@ final class TunnelDiagnosticsStore: @unchecked Sendable { reconnectAttempt: Int? = nil, pathSatisfied: Bool? = nil ) { + guard Self.providerRecordingEnabled else { return } let event = TunnelDiagnosticEvent( timestamp: Self.now(), source: "provider", @@ -184,6 +214,7 @@ final class TunnelDiagnosticsStore: @unchecked Sendable { } func writeHeartbeat(_ heartbeat: TunnelProviderHeartbeat) { + guard Self.providerRecordingEnabled else { return } writeJSON(heartbeat, to: .providerHeartbeat) } diff --git a/project.yml b/project.yml index 83918a1..1fd1a6b 100644 --- a/project.yml +++ b/project.yml @@ -32,6 +32,7 @@ settings: configs: Debug: debug Release: release + Measurement: release packages: FptnShared: @@ -172,6 +173,13 @@ targets: PRODUCT_BUNDLE_IDENTIFIER: net.mrmidi.FptnVPN.FptnVPNTunnel FRAMEWORK_SEARCH_PATHS: $(PROJECT_DIR)/FptnVPN/Cpp SWIFT_OBJC_BRIDGING_HEADER: FptnVPNTunnel/Cpp/FptnVPNTunnel-Bridging-Header.h + configs: + # PR-1 (Measurement Safety): the Measurement configuration disables + # legacy JSONL diagnostics, forces warning-only logging, and uses a + # memory-only telemetry path. Build with -configuration Measurement + # to produce a clean baseline for native memory profiling. + Measurement: + SWIFT_ACTIVE_COMPILATION_CONDITIONS: $(inherited) FPTN_MEASUREMENT_BUILD info: path: FptnVPNTunnel/Info.plist entitlements: