Document 3-second blocking sleep in WPS disconnect flow

NETWORK_DELAY_ANALYSIS.md

@@ -0,0 +1,193 @@
+# Network Delay Analysis for NetworkManager v1.13.0
+
+## Executive Summary
+
+This document analyzes the changeset introduced in NetworkManager v1.13.0 (released 2026-01-30) to identify potential network delays. The analysis focuses on three key features:
+1. IPv4LL support for Ethernet and WiFi interfaces
+2. WiFi enablement upon migration
+3. GCancellable implementation to fix async WiFi connection crashes
+
+## Changeset Overview
+
+According to the CHANGELOG.md, version 1.13.0 introduced:
+
+```
+### Added
+- Implemented IPv4LL support for both Ethernet and WiFi interfaces
+- Implemented logic to enable WiFi upon migration
+- Fixed crash of Thunder Plugin upon Async WiFiConnect by using GCancellable
+```
+
+## Detailed Findings
+
+### 1. GCancellable Implementation (Crash Fix)
+
+**Purpose:** Fix crashes during asynchronous WiFi connection operations
+
+**Location:** `plugin/gnome/NetworkManagerGnomeWIFI.h` and `.cpp`
+
+**Implementation Details:**
+- New member variable: `GCancellable *m_cancellable` (line 105 in header)
+- Thread-safe mutex protection: `std::mutex m_cancellableMutex` (line 106 in header)
+- Proper lifecycle management:
+  - Created in `createClientNewConnection()` (NetworkManagerGnomeWIFI.cpp, lines 66-69)
+  - Cancelled in `deleteClientConnection()` (NetworkManagerGnomeWIFI.cpp, lines 77-84)
+
+**Network Delay Impact:**
+- **✅ MINIMAL IMPACT** - GCancellable is properly implemented with async callbacks
+- No blocking operations introduced
+- Actually improves reliability by preventing crashes during async operations
+- Allows proper cancellation of pending operations without blocking
+
+### 2. WiFi Migration Logic
+
+**Purpose:** Enable WiFi automatically during system migration
+
+**Location:** `plugin/gnome/NetworkManagerGnomeWIFI.cpp`
+
+**Key Method:** `activateKnownConnection()` (lines ~800-930)
+
+**Implementation Details:**
+- Activates known WiFi connections during migration
+- Multiple synchronous wait operations on main loop
+- Sets autoconnect to true for saved connections
+
+**Potential Delay Points:**
+```cpp
+// Line 830, 898, 918: Multiple blocking wait() calls
+wait(m_loop);  // Default timeout: 10 seconds (line 63 in header)
+```
+
+**Network Delay Impact:**
+- **⚠️ MODERATE IMPACT** - Multiple blocking waits during migration
+- Each `wait()` call has a default 10-second timeout
+- Could delay network availability by 10-30 seconds during migration
+- Migration process is synchronous, blocking other network operations
+
+### 3. IPv4LL (Link-Local Addressing) Support
+
+**Purpose:** Provide automatic IP addressing when DHCP is unavailable
+
+**Current Status:**
+- **❌ NOT FOUND** - No explicit IPv4LL implementation detected in codebase
+- Standard DHCP configuration present (lines 606, 614-618 in NetworkManagerGnomeWIFI.cpp)
+- IPv4 change callbacks exist but no link-local fallback logic visible
+- DHCP timeout set to `INT32_MAX` (infinite) on line 614
+
+**Network Delay Impact:**
+- **⚠️ POTENTIAL IMPACT** - If IPv4LL was added but not visible in current code
+- Link-local addressing typically adds 2-3 seconds for address probing
+- Could cause initial connection delays when DHCP is slow/unavailable
+
+## Critical Delay Issues Discovered
+
+### Issue #1: WPS Process Synchronous Sleep
+
+**Location:** `plugin/gnome/NetworkManagerGnomeWIFI.cpp`
+
+**Problem:**
+```cpp
+// Line 1683: Blocking 3-second sleep during WPS disconnect
+sleep(3);  // wait for 3 sec to complete disconnect process
+
+// Line 1574: Additional sleep in retry loop
+sleep(WPS_RETRY_WAIT_IN_MS);  // 10 seconds defined in header
+```
+
+**Impact:**
+- **🔴 HIGH IMPACT** - Blocks network thread for 3+ seconds
+- Can accumulate to 30+ seconds with retries (WPS_RETRY_COUNT = 10)
+- Affects all network operations during WPS connect/disconnect
+
+### Issue #2: Default Timeout Values
+
+**Location:** `plugin/gnome/NetworkManagerGnomeWIFI.h`
+
+**Constants:**
+```cpp
+#define WPS_RETRY_WAIT_IN_MS        10  // 10 sec
+#define WPS_RETRY_COUNT             10
+#define WPS_PROCESS_WAIT_IN_MS      5
+
+bool wait(GMainLoop *loop, int timeOutMs = 10000); // 10 second default
+```
+
+**Impact:**
+- **⚠️ MODERATE IMPACT** - 10-second default timeouts
+- Can cause perceived delays during normal WiFi operations
+- WPS operations can take up to 100 seconds (10 retries × 10 seconds)
+
+### Issue #3: Multiple Blocking Wait Calls
+
+**Location:** Throughout `plugin/gnome/NetworkManagerGnomeWIFI.cpp`
+
+**Occurrences:**
+- Line 330: `wait(m_loop);` - WiFi disconnect
+- Line 830: `wait(m_loop);` - Known connection activation
+- Line 898: `wait(m_loop);` - Connection update
+- Line 918: `wait(m_loop);` - Connection activation
+- Multiple other instances in connect/disconnect flows
+
+**Impact:**
+- **⚠️ MODERATE IMPACT** - Sequential blocking operations
+- Each can take up to 10 seconds
+- No parallel operation support during migration
+
+## Recommendations
+
+### High Priority
+
+1. **Replace Synchronous Sleeps:**
+   - Replace `sleep(3)` with async callback mechanisms
+   - Use g_timeout_add() for non-blocking delays
+   - Implement proper state machine for WPS operations
+
+2. **Reduce Default Timeouts:**
+   - Lower default timeout from 10 seconds to 5 seconds for interactive operations
+   - Implement progressive timeout strategies (fast timeout, then retry with longer timeout)
+
+3. **Implement IPv4LL Properly:**
+   - If IPv4LL is claimed in v1.13.0, verify implementation exists
+   - Document IPv4LL configuration and timeout behavior
+   - Ensure link-local addressing doesn't block DHCP attempts
+
+### Medium Priority
+
+1. **Make Migration Async:**
+   - Convert migration logic to non-blocking async operations
+   - Implement callback-based activation for known connections
+   - Allow partial network availability during migration
+
+2. **Add Timeout Configuration:**
+   - Make timeout values configurable via configuration file
+   - Different timeout values for different operation types
+   - Allow override for low-latency vs. high-reliability scenarios
+
+### Low Priority
+
+1. **Optimize WPS Retry Logic:**
+   - Implement exponential backoff instead of fixed delays
+   - Add early termination on unrecoverable errors
+   - Reduce retry count or make it configurable
+
+## Conclusion
+
+The v1.13.0 changeset introduces **potential network delays** primarily through:
+
+1. **WiFi Migration Logic** - Synchronous blocking operations with 10+ second timeouts
+2. **WPS Operations** - Multiple blocking sleeps totaling 3-30+ seconds
+3. **Missing IPv4LL Implementation** - Claimed feature not visible in code
+
+The **GCancellable implementation** is well-designed and does NOT introduce delays; it actually improves reliability.
+
+### Overall Risk Assessment:
+- **Migration scenarios:** 10-30 second delays possible ⚠️
+- **WPS operations:** 3-100 second delays possible 🔴
+- **Normal WiFi connect:** 10 second timeout delays possible ⚠️
+- **Crash prevention (GCancellable):** No delay impact ✅
+
+### Recommended Actions:
+1. Review and optimize WPS sleep() calls immediately
+2. Make migration logic asynchronous
+3. Verify IPv4LL implementation or update changelog
+4. Reduce default timeout values for better user experience

SLEEP_3SEC_DELAY.md

@@ -0,0 +1,105 @@
+# 3-Second Network Delay - Sleep Call Location
+
+## Summary
+A **blocking sleep(3)** call causes a **3-second network delay** during WPS (Wi-Fi Protected Setup) operations.
+
+## Exact Location
+
+**File:** `plugin/gnome/NetworkManagerGnomeWIFI.cpp`  
+**Line:** `1683`  
+**Function:** `wifiManager::wpsProcess()`
+
+## Code Context
+
+```cpp
+// Line 1673-1684
+NMDeviceState state = nm_device_get_state(wifidevice);
+if(state > NM_DEVICE_STATE_DISCONNECTED)
+{
+    NMLOG_INFO("stopping the ongoing Wi-Fi connection");
+    // some other ssid connected or connecting; wps need a disconnected wifi state
+    nm_device_disconnect(wifidevice, NULL,  &error);
+    if (error) {
+        NMLOG_ERROR("disconnect connection failed %s", error->message);
+        g_error_free(error);
+    }
+    sleep(3); // wait for 3 sec to complete disconnect process
+}
+```
+
+## When This Delay Occurs
+
+This 3-second blocking sleep happens when:
+1. **WPS (Wi-Fi Protected Setup)** connection process is running
+2. Another WiFi network is already connected or connecting
+3. The system needs to disconnect from the current network before WPS can proceed
+
+## Impact
+
+### Severity: 🔴 **HIGH**
+
+- **Duration:** Blocks network thread for exactly **3 seconds**
+- **Frequency:** Can occur on **every WPS retry** (up to 10 retries)
+- **Cumulative Impact:** Potentially **30 seconds total delay** in worst case
+- **Blocking Nature:** This is a **synchronous sleep** - completely blocks the thread
+- **User Experience:** WiFi becomes unresponsive during this time
+
+## Why This Is Problematic
+
+1. **Synchronous Blocking:** The `sleep(3)` call blocks the entire thread, preventing any network operations
+2. **No Async Handling:** Unlike other operations that use callbacks, this is a hard sleep
+3. **Fixed Duration:** Always waits 3 seconds regardless of actual disconnect time
+4. **Multiple Occurrences:** Can happen multiple times during WPS retry loop
+
+## Related Code Constants
+
+**File:** `plugin/gnome/NetworkManagerGnomeWIFI.h`  
+**Lines:** `35-37`
+
+```cpp
+#define WPS_RETRY_WAIT_IN_MS        10 // 10 sec
+#define WPS_RETRY_COUNT             10
+#define WPS_PROCESS_WAIT_IN_MS      5
+```
+
+## Full Context - WPS Process Flow
+
+The `wpsProcess()` function (starting at line 1553) includes:
+- A retry loop that runs up to 10 times (WPS_RETRY_COUNT = 10)
+- Each retry sleeps for 10 seconds (WPS_RETRY_WAIT_IN_MS)
+- If disconnection is needed, adds this additional 3-second sleep
+
+**Total potential delay in worst case:**
+- 10 retries × 10 seconds = 100 seconds (retry delays)
+- Plus up to 10 × 3 seconds = 30 seconds (disconnect delays)
+- **Maximum: 130 seconds of blocking delays**
+
+## Recommended Solution
+
+Replace the synchronous `sleep(3)` with:
+
+1. **Async callback approach:** Wait for actual disconnect event
+2. **Event-driven logic:** Use NetworkManager's state change signals
+3. **Timeout with polling:** Use `g_timeout_add()` with shorter intervals
+4. **Conditional check:** Verify disconnect completed before proceeding
+
+Example replacement:
+```cpp
+// Instead of: sleep(3);
+// Use async approach with callback or event monitoring
+GSource *source = g_timeout_source_new(100);  // Check every 100ms
+// Set callback to monitor disconnect state
+// Continue when state == NM_DEVICE_STATE_DISCONNECTED or timeout after 3s
+```
+
+## Additional Sleep Calls in Same File
+
+For complete picture, other sleep/delay calls:
+- **Line 1574:** `sleep(WPS_RETRY_WAIT_IN_MS);` - 10 second sleep per retry
+- **Line 1913:** `g_usleep(500 * 1000);` - 500ms microsleep (less critical)
+
+## References
+
+- Main analysis document: [NETWORK_DELAY_ANALYSIS.md](NETWORK_DELAY_ANALYSIS.md)
+- WPS implementation: `plugin/gnome/NetworkManagerGnomeWIFI.cpp:1553-1800`
+- Related issue: WPS disconnect synchronization