Stop marking unmetered networks as metered while the VPN runs - #960
Merged
Merged
Conversation
Android defaults a VPN network to metered, so the metered flag has to be set explicitly. We passed a snapshot of Util.isMeteredNetwork(), which is wrong twice over: - ConnectivityManager.isActiveNetworkMetered() returns true when there is no active network at all, so a tunnel established before Wi-Fi associates (boot start, always-on VPN) is born metered; - the flag only reaches apps through establish(), and Builder.equals() never compared it, so a metered reload took the "Native restart" shortcut and the stale value survived for the life of the tunnel. Apps that ask the system whether the current network is metered — K-9 Mail, Syncthing — then hold back on any Wi-Fi for as long as TC runs (#959). Pass false instead, which does not force the network unmetered: it tells the platform to inherit meteredness from the underlying networks, so the VPN tracks the physical network as it changes, with no snapshot to go stale. Also track the flag in Builder and compare it, so a future change of the value does force a real re-establish (part of #763). Closes #959 Claude-Session: https://claude.ai/code/session_01184pQBPQMMwWef1H7DmgJA
kasnder
marked this pull request as ready for review
September 18, 2026 19:14
kasnder
added a commit
that referenced
this pull request
Sep 19, 2026
Builder.equals decides whether reload() replaces the live tun or takes the "Native restart" shortcut, so any field it does not capture cannot reach a running interface. Two were still missing after #960 fixed the metered flag: - search domains (addSearchDomain); - the carrier ePDG exclusions (excludeRoute, API 33+). These are re-resolved on every rebuild behind a 1.5s timeout, so a first establish whose lookup timed out was never replaced by a later rebuild that resolved them, and Wi-Fi calling stayed broken until some unrelated change forced a real replacement. Both are now recorded by overriding the corresponding Builder methods and compared like the existing lists. Also: two offline builders (both networkInfo null) now compare equal instead of forcing a needless replacement on every reload while there is no active network, and the cast to Builder is guarded by an instanceof check rather than relying on a following null check. The extra interface replacements this produces go through VpnReplacementSequencer, which was not in place when the issue was first triaged. Refs #763 Claude-Session: https://claude.ai/code/session_016eKiHiWiQwMQosDXj8qY4s Co-authored-by: Claude <noreply@anthropic.com>
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 #959, and the metered half of #763.
The bug
Android defaults a VPN network to metered, so
VpnService.Builder.setMeteredhas to be called explicitly. We passed a snapshot ofUtil.isMeteredNetwork(), which is wrong twice over:ConnectivityManager.isActiveNetworkMetered()returnstruewhen there is no active network at all (AOSP's conservative default), so a tunnel established before Wi-Fi associates — a boot start, an always-on VPN, a start during a handover — is born metered.establish(), andBuilder.equalsnever compared it.NetworkReloadPolicydoes fireREASON_METERED_CHANGED, but the two builders compare equal, soreload()takes the "Native restart" branch and the interface is not replaced. The stale value then survives for the life of the tunnel.Apps that ask the system whether the current network is metered — K-9 Mail, Syncthing — hold back on any Wi-Fi for as long as TrackerControl runs.
This is not a recent regression: the
setMetered(Util.isMeteredNetwork(this))call dates back to the 2020 NetGuard import and is unchanged across every release since.The change
getBuilderandgetBlockingBuilderpasssetMetered(false). That does not force the network unmetered: the platform documentsfalseas "inherit meteredness from the underlying networks", so the VPN tracks the physical network as it changes and there is no snapshot left to go stale.Buildernow records the flag andBuilder.equalscompares it, so if the value ever becomes conditional again, a change forces a real re-establish instead of the native-restart shortcut. The rest of Builder.equals ignores metered flag and search domains, leaving stale VPN interfaces #763 (search domains, ePDGexcludeRoutes) is untouched and that issue stays open.Util.isMeteredNetworkis still used for rule computation ingetAllowedRules, so the metered reload trigger keeps earning its place.Testing
Not compiled locally: this container has no Android SDK, and Maven Central rate-limited (HTTP 429) the Gradle plugin classpath through most of the session. Relying on CI for
compileGithubDebugJavaWithJavac, the unit tests and lint. On-device confirmation still wanted: with the tunnel up on unmetered Wi-Fi, Syncthing-fork's status should no longer report a metered connection, and it should flip to metered on mobile data.https://claude.ai/code/session_01184pQBPQMMwWef1H7DmgJA
Generated by Claude Code