Improve connectivity detection and proxy handling#14764
Open
valldrac wants to merge 5 commits into
Open
Conversation
Replace NetworkConnectionListener with InternetConnectivityMonitor, which uses a coroutine-based Flow to aggregate per-network state and derive a single ConnectivityState. The monitor tracks all networks instead of relying on the default network callback, so VPN and underlying networks are considered. Co-authored-by: S1m <31284753+p1gp1g@users.noreply.github.com>
The ProxySelector approach is simpler and handles PAC transparently. On proxy change detection in IncomingMessageObserver, reconfigure the libsignal network proxy and reconnect the websockets, instead of resetting the entire network module.
It eliminates the race where an initial drainPermits() in the wait loop could silently discard a valid wake-up signal.
Following the previous refactor, the ReentrantLock is no longer needed. The only remaining shared state is appVisible and lastInteractionTime, which are replaced with an AppState data class to keep reads consistent.
Open
4 tasks
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
1 participant
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.
This PR is an attempt to address #14528.
The main goal is to make Internet connectivity detection more reliable under VPNs and fast network transitions, as discussed in the linked issue.
The core change is a new
InternetConnectivityMonitor, which replacesNetworkConnectionListener. The new monitor tracks all active networks, including VPNs, and derives a single connectivity state from that aggregate view. This is intended to fix cases whereIncomingMessageObservercan make the wrong connection decision under always-on VPNs and kill switch.Also, the previous code was mixing asynchronous connectivity updates from
NetworkCallbackwith synchronous connectivity checks via deprecated APIs. Our assumption is that this could lead to races and inconsistent connection decisions when networks switches quickly.Proxy handling is also reworked. Proxy change events are now detected via
PROXY_CHANGE_ACTIONsystem broadcast. We removed the old proxy lookup path and its IPC overhead fromSignalServiceNetworkAccess. Proxy resolution now usesProxySelector, which handles PAC and exclusion rules transparently.Summary of changes:
NetworkConnectionListenerwithInternetConnectivityMonitorProxySelectorwaitForConnectionNecessary()IncomingMessageObserverA few notes:
We split the work into smaller commits so each one can be reviewed independently, but ideally in order.
Closes #14528.