Skip to content

Stop marking unmetered networks as metered while the VPN runs - #960

Merged
kasnder merged 1 commit into
masterfrom
claude/practical-lovelace-cfpgt7
Sep 18, 2026
Merged

kasnder merged 1 commit into
masterfrom
claude/practical-lovelace-cfpgt7

Conversation

@kasnder

@kasnder kasnder commented Sep 18, 2026

Copy link
Copy Markdown
Member

Fixes #959, and the metered half of #763.

The bug

Android defaults a VPN network to metered, so VpnService.Builder.setMetered has to be called 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 (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.
  • The flag only reaches apps through establish(), and Builder.equals never compared it. NetworkReloadPolicy does fire REASON_METERED_CHANGED, but the two builders compare equal, so reload() 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

  • getBuilder and getBlockingBuilder pass setMetered(false). That does not force the network unmetered: the platform documents false as "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.
  • Builder now records the flag and Builder.equals compares 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, ePDG excludeRoutes) is untouched and that issue stays open.

Util.isMeteredNetwork is still used for rule computation in getAllowedRules, 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

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
kasnder marked this pull request as ready for review September 18, 2026 19:14
@kasnder
kasnder merged commit c4e19f3 into master Sep 18, 2026
3 checks passed
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

running TC marks any wi-fi as a metered connection

2 participants