Use NWPathMonitor for Apple reachability#1431
Open
bmehta001 wants to merge 26 commits into
Open
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
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.
Annotate the modern-path helpers (`ODWModernPathIsReachable`, `ensureModernPathMonitor`, `awaitModernPathSnapshot`, `handleModernPathUpdate:`, `notifyModernPathChange`) with `API_AVAILABLE(macos(10.14), ios(12.0))` so that compiling against an older deployment target no longer triggers `-Wunguarded-availability-new` errors under `-Werror`. Also fix a latent bug in `isReachableViaWWAN` where the modern-path snapshot was computed before the `if (@available(...))` guard. On macOS 10.10 / iOS 10.0 deployment targets running on a host that lacks NWPathMonitor, this would invoke unavailable Network framework APIs. The modern-path branch is now only taken inside the `@available` block on legacy builds, mirroring the structure used in `isReachableViaWiFi`. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
`-handleModernPathUpdate:` and `-awaitModernPathSnapshot` previously read `self.initialPathSemaphore` more than once across the nil-check and the matching `dispatch_semaphore_signal`/`dispatch_semaphore_wait` call. `-stopNotifier` clears the property from an arbitrary thread, so the second read could observe nil and pass it into libdispatch (which crashes on a nil semaphore). Capture the semaphore into a strong local once. Under ARC the local retains the dispatch_semaphore_t for the duration of the call, so a concurrent stop will no longer race the signal/wait. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve host/address-specific reachability semantics with SCNetworkReachability while keeping Network.framework for general path monitoring. Avoid waiting for the first NWPathMonitor update on the monitor queue, and use platform-specific Apple availability gates. Files changed: - third_party/Reachability/ODWReachability.h - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Leave the Network.framework status initialization in modern monitor setup so older Apple deployment-target builds do not touch an availability-gated symbol from init. Files changed: - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove tvOS and watchOS deployment-target handling because the SDK does not support those targets; keep the availability gates focused on iOS and macOS. Files changed: - third_party/Reachability/ODWReachability.h - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Serialize NWPathMonitor initialization and use a dispatch group so every caller waiting for the first path snapshot is released when the snapshot arrives or monitoring stops. Files changed: - third_party/Reachability/ODWReachability.m - lib/pal/posix/NetworkInformationImpl.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot: host:port inputs were parsed as a scheme with no host. Parse by prepending https:// before NSURLComponents when no explicit scheme separator is present. Copilot: invalid.hostname is syntactically valid under SCNetworkReachabilityCreateWithName. Update tests to distinguish malformed host URLs from unresolved hostnames. Files changed: - third_party/Reachability/ODWReachability.m - tests/unittests/obj-c/ODWReachabilityTests.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot: ODWReachability.m could look non-ARC under CMake because ARC was only set in CXX flags. Compile the vendored Objective-C source with -fobjc-arc explicitly. Copilot: rename the unresolved-hostname test to describe constructor acceptance rather than DNS reachability. Files changed: - lib/CMakeLists.txt - tests/unittests/obj-c/ODWReachabilityTests.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot: delete the dead URLSession response helper left behind after removing endpoint probes. Copilot: make modern startNotifier match legacy behavior by wiring callbacks and returning without waiting for the first NWPathMonitor snapshot. Copilot: add a start/stop notifier test that verifies startup returns promptly. Files changed: - third_party/Reachability/ODWReachability.m - tests/unittests/obj-c/ODWReachabilityTests.mm Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the access-specifier indentation in NetworkInformationImpl.mm to the 4-space convention used on main. Restore the trailing blank line at the end of lib/CMakeLists.txt accidentally dropped during the ARC compile-flags patch. Realign one stray line in ODWReachability.m reachabilityFlags() to match surrounding column alignment. Files changed: - lib/CMakeLists.txt - lib/pal/posix/NetworkInformationImpl.mm - third_party/Reachability/ODWReachability.m Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Align build-ios.sh with the README support matrix (iOS 12+) so default builds compile out the legacy SCNetworkReachability code path that ODW_LEGACY_REACHABILITY_REQUIRED gates. Borrowed from microsoft#1371. Files changed: - build-ios.sh Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ODWReachability is internal-only, so keep the SDK on direct NWPathMonitor instead of maintaining a compatibility wrapper around the vendored Reachability snapshot. Remove the vendored Reachability sources, tests, CMake wiring, Xcode references, and OSS manifest entries. Unsupported pre-iOS-12 / pre-macOS-10.14 runtimes now leave network state as Unknown instead of using SCNetworkReachability. Files changed: - lib/pal/posix/NetworkInformationImpl.mm - lib/CMakeLists.txt - tests/unittests/unittests-ios.xcodeproj/project.pbxproj - tests/unittests/obj-c/ODWReachabilityTests.mm - third_party/Reachability/* - docs/List-of-OSS-Components.md - third_party/cgmanifest.json Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
changed the title
denny-msft
approved these changes
May 28, 2026
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.
Fixes #1370. Supersedes #1371.
Summary
This PR fixes the iOS 18 reachability crash by removing the internal
ODWReachabilityhelper and moving the SDK's Apple network detection directly toNWPathMonitor.ODWReachabilitywas internal-only, so we do not need to preserve its compatibility wrapper or vendored third-party snapshot. This keeps one Apple reachability implementation in the SDK and avoids the extra notifier/waiter/race surface from maintaining two backends.What changed
NetworkInformationImpl.mmnow owns Apple reachability directly throughnw_path_monitor_create().third_party/Reachabilitysnapshot, its README, and its iOS unit tests were removed.ODWReachabilitywere removed.build-ios.shnow defaults to iOS 12.0, matching the README support matrix.third_party/cgmanifest.jsonno longer list Tony Million Reachability.Compatibility notes
NetworkInformationImplleaves network type/cost asUnknowninstead of usingSCNetworkReachability.Validation
third_party/cgmanifest.jsonwas validated after removing the Reachability entry.