🔥 CRITICAL: Fix Memory Leaks and Retain Cycles in AR Processing

[TangoEcho]

Problem Description

Critical memory management issues causing app crashes and poor performance, particularly in AR visualization and speed test operations.

Current Issues

1. AR Frame Processing Retain Cycles (ARVisualizationManager.swift:936-967)

   • Strong references to ARFrame in closures and timer blocks
   • Frames not properly released, causing memory pressure
   • Potential crash during extended AR sessions

2. Speed Test Memory Leaks (WiFiSurveyManager.swift:197-307)

   • URLSession tasks holding strong references
   • Completion handlers not properly released
   • Background thread operations not cleaned up

3. Node Pool Management Issues (ARVisualizationManager.swift:295-331)

   • SCNNode objects accumulating without proper cleanup
   • Collections modified while being enumerated
   • Missing removeAllAnimations() calls

Affected Files

• RoomPlanSimple/ARVisualizationManager.swift (lines 295-331, 936-967)
• RoomPlanSimple/WiFiSurveyManager.swift (lines 197-307)

Current Problematic Code

AR Frame Processing

// PROBLEMATIC CODE:
guard let currentFrame = sceneView.session.currentFrame else { return }
// Frame reference held in closure without proper cleanup
Timer.scheduledTimer(withTimeInterval: 2.0, repeats: true) { timer in
    // Strong reference to frame in repeating timer
    let position = currentFrame.camera.transform
}

Speed Test Operations

// PROBLEMATIC CODE:
let task = session.downloadTask(with: request) { [weak self] tempURL, response, error in
    // Multiple nested DispatchQueue.main.async calls
    DispatchQueue.main.async {
        DispatchQueue.main.asyncAfter(deadline: .now() + 1.0) {
            // Potential retain cycle with nested closures
        }
    }
}

Solution

1. Fix AR Frame Processing

// SOLUTION: Extract data immediately, don't hold frame reference
func processCurrentFrame() {
    guard let sceneView = sceneView else { return }
    let cameraTransform = sceneView.session.currentFrame?.camera.transform
    
    // Use weak self in timer
    Timer.scheduledTimer(withTimeInterval: 2.0, repeats: true) { [weak self] timer in
        guard let self = self else {
            timer.invalidate()
            return
        }
        // Use extracted data, not frame reference
    }
}

2. Fix Speed Test Memory Management

// SOLUTION: Proper cleanup and weak references
func performSpeedTest() async throws -> Double {
    return try await withCheckedThrowingContinuation { [weak self] continuation in
        guard let self = self else { 
            continuation.resume(throwing: SpeedTestError.cancelled)
            return 
        }
        
        let task = URLSession.shared.downloadTask(with: request) { tempURL, response, error in
            // Single cleanup point
            defer { self.cleanup() }
            // Handle result...
        }
        task.resume()
    }
}

3. Fix Node Pool Management

// SOLUTION: Thread-safe node management
actor ARNodeManager {
    private var nodePool: [SCNNode] = []
    private var activeNodes: [SCNNode] = []
    
    func addNode(_ node: SCNNode) async {
        if activeNodes.count >= maxNodes {
            let oldNode = activeNodes.removeFirst()
            await cleanupNode(oldNode)
        }
        activeNodes.append(node)
    }
    
    private func cleanupNode(_ node: SCNNode) async {
        await MainActor.run {
            node.removeAllAnimations()
            node.removeFromParentNode()
            nodePool.append(node)
        }
    }
}

Implementation Steps

1. Fix AR Processing (~2 days)

   • Replace frame references with extracted data
   • Add proper timer cleanup
   • Use weak references in all closures

2. Fix Speed Test Leaks (~1 day)

   • Modernize to async/await pattern
   • Add proper cleanup methods
   • Remove nested dispatch calls

3. Implement Thread-Safe Collections (~2 days)

   • Use actors for shared state
   • Add proper synchronization
   • Implement safe enumeration patterns

4. Add Memory Monitoring (~1 day)

   • Add memory usage logging
   • Implement leak detection
   • Create automated memory tests

Acceptance Criteria

• No retain cycles detected by Instruments
• Memory usage stable during extended AR sessions
• Proper cleanup when leaving AR mode
• Speed tests don't accumulate memory
• Collections thread-safe under concurrent access
• Memory monitoring and alerting in place

Testing Strategy

Memory Testing

func testARMemoryUsage() {
    // Measure memory before AR session
    let initialMemory = getMemoryUsage()
    
    // Run extended AR session
    for _ in 0..<100 {
        addARVisualization()
    }
    
    // Clear AR session
    clearARSession()
    
    // Verify memory returned to baseline
    let finalMemory = getMemoryUsage()
    XCTAssertLessThan(finalMemory - initialMemory, 10_000_000) // <10MB leak tolerance
}

Benefits

• Stability: Eliminates crashes during extended use
• Performance: Better memory efficiency and frame rates
• User Experience: Smooth AR performance without stuttering
• App Store Review: Prevents rejection due to memory issues

Estimated Effort

• Priority: 🔥 Critical
• Effort: 1 week
• Impact: High - Critical for app stability

Dependencies

• Requires Xcode Instruments for memory testing
• Should be completed before architectural refactoring