Use NWPathMonitor for modern Apple reachability with legacy fallback#1431
Open
bmehta001 wants to merge 6 commits intomicrosoft:mainfrom
Open
Use NWPathMonitor for modern Apple reachability with legacy fallback#1431bmehta001 wants to merge 6 commits intomicrosoft:mainfrom
bmehta001 wants to merge 6 commits intomicrosoft:mainfrom
Conversation
Replace the deprecated SCNetworkReachability APIs with NWPathMonitor for modern Apple deployment targets (iOS 12+, macOS 10.14+). The legacy SCNetworkReachability path is retained behind a compile-time check for older targets. Changes: - NetworkInformationImpl.mm: refactor to use NWPathMonitor as the primary reachability mechanism, with SCNetworkReachability as fallback for older deployment targets only - ODWReachability.h/m: add NWPathMonitor-based implementation gated on availability, keeping SCNetworkReachability for backward compatibility - Remove dead private header imports from tests This eliminates the -Wdeprecated-declarations build failures on Xcode 26.4+ without needing pragma suppressions. Fixes microsoft#1425 Co-authored-by: Copilot <[email protected]>
PR microsoft#1431 should not carry changes in ODWReachabilityTests.mm. Restore the socket header imports so the branch only contains the reachability implementation changes. Files changed: - tests/unittests/obj-c/ODWReachabilityTests.mm Co-authored-by: Copilot <[email protected]>
Keep PR microsoft#1431 focused on the reachability implementation changes by reverting the top-of-file/header-area edits in ODWReachability.m. Files changed: - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Apple-side reachability/network detection code paths to avoid SCNetworkReachability deprecation build failures by preferring modern APIs on newer deployment targets while retaining a legacy fallback for older targets.
Changes:
- Adds a compile-time deployment-target gate (
ODW_LEGACY_REACHABILITY_REQUIRED) to exclude legacy SCNetworkReachability code from modern builds. - Refactors
NetworkInformationImpl.mmto split modern (NWPathMonitor) vs legacy (ODWReachability notification) setup paths and compile out legacy members when not needed. - Wraps multiple
ODWReachability.mcode paths with#if ODW_LEGACY_REACHABILITY_REQUIREDto avoid compiling deprecated SCNetworkReachability usage for modern targets.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| third_party/Reachability/ODWReachability.h | Introduces ODW_LEGACY_REACHABILITY_REQUIRED macro to compile out legacy reachability code on modern deployment targets. |
| third_party/Reachability/ODWReachability.m | Adds compile-time gating around legacy SCNetworkReachability branches to avoid deprecation warnings/errors on modern targets. |
| lib/pal/posix/NetworkInformationImpl.mm | Refactors network detection setup into modern (NWPathMonitor) vs legacy (ODWReachability) implementations and gates legacy members/functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
Make the modern ODWReachability WWAN path explicit so iOS 12+ builds unambiguously use the NWPathMonitor-backed state, while the legacy SCNetworkReachability fallback remains only for older Apple deployment targets. Co-authored-by: Copilot <[email protected]>
bmehta001
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Modern reachability should not synchronously resolve DNS or reject the generic internet reachability address, and the path monitor context must not hold a stale raw owner pointer. Also avoid blocking the main thread while waiting for the first NWPathMonitor snapshot. Files changed: - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <[email protected]>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+60
to
+74
| @property (nonatomic, strong) nw_path_monitor_t pathMonitor; | ||
| @property (nonatomic, strong) ODWReachabilityMonitorContext *pathMonitorContext; | ||
| @property (nonatomic, strong) dispatch_semaphore_t initialPathSemaphore; | ||
| @property (nonatomic, assign) nw_path_status_t currentPathStatus; | ||
| @property (nonatomic, assign) BOOL currentPathUsesWiFi; | ||
| @property (nonatomic, assign) BOOL currentPathUsesWWAN; | ||
| @property (nonatomic, assign) BOOL hasObservedPath; | ||
| @property (nonatomic, assign) BOOL monitorLocalWiFiOnly; | ||
|
|
||
| -(void)reachabilityChanged:(SCNetworkReachabilityFlags)flags; | ||
| -(BOOL)isReachableWithFlags:(SCNetworkReachabilityFlags)flags; | ||
| -(BOOL)ensureModernPathMonitor; | ||
| -(BOOL)awaitModernPathSnapshot; | ||
| -(void)handleModernPathUpdate:(nw_path_t)path; | ||
| -(void)notifyModernPathChange; |
Comment on lines
+97
to
100
| static BOOL ODWModernPathIsReachable(nw_path_status_t status) | ||
| { | ||
| return status == nw_path_status_satisfied || status == nw_path_status_satisfiable; | ||
| } |
Comment on lines
291
to
+294
| self.reachableOnWWAN = YES; | ||
| self.reachabilityRef = ref; | ||
| self.reachabilitySerialQueue = dispatch_queue_create("com.tonymillion.reachability", NULL); | ||
| self.currentPathStatus = nw_path_status_invalid; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Use NWPathMonitor for the modern Apple reachability path while retaining SCNetworkReachability as a fallback for older deployment targets.
Changes
This removes the Xcode 26.4+ deprecated reachability build failures without relying on pragma suppressions in the modern Apple path.
Fixes #1425