Link diagnostics that can see a media-only outage, an offline VPN with an owner, and a wireless stack that stops running under USB - #867
Draft
o-jcardenass wants to merge 5 commits into
Conversation
Five captures from a 1 GB MediaTek head unit put the same fault in front of three instruments and none of them reported it. A 2x2 of codec against the head unit's WiFi station being joined to a network shows the station is the variable and the codec is not: joined, no dropout longer than 0.65s in 138s and zero audio underruns on H.265, and on H.264 a 69s clean stretch with zero underruns; unjoined, the picture gone for four to eight seconds every ten and a half, thirty underruns in two minutes, both codecs alike. What follows is what the app needed in order to have said so itself. LinkGapMonitor measures silence on any channel against 1200ms, and the phone sends a CONTROL Ping Request about once a second for the life of the session, so a link carrying nothing but pings scores healthy while both media channels are gone. It printed twice across the five logs, both times naming a gap of 1.6-1.8s, including in windows that each held three six-second blackouts. So the same measurement now runs per series - the all-channel monitor untouched, plus video and the three audio sinks - because reading the three together is what separates a dead radio, which silences all of them, from a stalled media path, which is what the captures show: video and audio dying in the same window twelve times out of twelve while control ran on. The media series also need more than a gap count before they speak, and hardware answered that question rather than a design assumption. Three untouched minutes on a stationary Google Maps screen with no navigation printed four video-gap lines on a rig whose session was clean by every other measure. The assumption had been that Android Auto sends no video at all while nothing animates, one long silence per window, which a two-gaps-minimum floor would suppress. What it actually sends is a trickle: an isolated packet every few seconds, a clock tick or a location pulse, and every one of those arrivals closes a gap and opens the next. Measured: four windows, 2-5 gaps in each, dead=95%, 99%, 96%, 99%, intervals scattered from 3.3s to 17.9s. The floor suppressed none of it. What separates that from the fault is not how many gaps there are but how much picture runs between them. The reporter's waveform is 4-8s of silence every 10.5s with video at 45-55 fps in between - dead around 55-60%, and 22% on the milder capture - against 95-99% for a screen nobody is touching. Nothing measured has ever landed between, so the media series require both two gaps and a window no more than 85% dead. A window past that is not a stuttering picture, it is a stopped one, and a stopped picture already has instruments: the projection watchdog decides when to call it a lost connection and the decoder reports the rate. The link series keeps no ceiling, because a link that quiet really is dead. UplinkStallMonitor adds the half no inbound series can supply. The media channels are flow-controlled by max_unacked - twelve frames on video, thirty on each audio sink - so a head unit whose MediaAcks stop leaving the socket stalls video and audio together and leaves control alone, which is the observed shape. The send handler serves one socket from one thread, so the write's own duration is the time the uplink refused to drain. logStationCoexistence told the user that one radio serving both the station link and the group can stall video and audio together, and to disconnect the other network. It said so on a comparison it had just described as unavailable: WifiP2pGroup.getFrequency() arrived in API 29, so every unit below that reports the group frequency as zero and the advice went out anyway. On this unit it fired on all three station-joined runs and on neither unjoined one, and joined was the configuration that worked. It then fired again on a second rig with both frequencies known and 260 MHz apart, during ten minutes at 45-55 fps with no gap on any channel. That is two units now, and no measurement anywhere in which disconnecting the station helped, so StationCoexistencePolicy keeps a prescription for nothing: the retune is real and worth naming in a log, telling the user to act on it is not supported, and the line now describes and stops. The last change is not diagnostic. maybeRecoverFromDisplayStall rebuilt the projection view and the decoder in the middle of a link outage - about 1.7s more black screen plus a forced focus cycle, for a fault no rebuild can fix - and two defects in its long-frame counter made that inevitable. The baseline is never seeded: the check survives its gates only when the picture has been still for over two seconds while video arrived within the last one and a half, which happened once in two minutes on this link, and subtracting a baseline of zero charged the session's entire fourteen long frames to that one tick, tripping a floor of ten on its first evaluation. And the window counted slots rather than time, while the check returns early whenever video is not arriving, so five surviving ticks could span two minutes. Each slot now carries the clock reading it was written at, only slots inside the window are summed, and a tick with no recent baseline re-baselines and charges nothing. A consumer that is genuinely collapsing keeps ticking, because video keeps arriving throughout, and loses only its first tick; the escalation ladder and the frozen branch are untouched. Nothing reads either monitor's report.
…s off Two lines a reporter's capture needed and did not have. logStationCoexistence returned silently whenever the supplicant was not associated, so only one arm of the comparison ever printed. A capture with no coexistence line in it meant either "not joined to any other network" or "the read threw", and nothing could tell those apart. Whether the head unit's own station is joined to a network is the single variable that separated a clean session from one losing picture and sound every four to eight seconds on the unit behind this work: joined, no dropout over 0.65s in 138s and no audio underrun at all; unjoined, twelve blackouts and thirty underruns over a comparable window. Both codecs were clean joined and broken unjoined, so this is the axis that matters and it was the one the log could not carry. It cost this work directly. A test brief asserted the rig had no station association; the rig has been joined to a 5500 MHz network throughout, which made the unjoined arm of a planned run untestable and was only found when the joined arm printed something the brief said was impossible. StationCoexistencePolicy gains describeNotAssociated(), a new function rather than a flag on describe(): every existing branch presumes association, and a zero station frequency there already means "associated, frequency unreadable below Android 10", which must not collapse into "not associated". A test pins the property the change exists for, that the two arms are separable by a single grep. The dedupe key now carries the association state alongside the group, so a station that drops or joins part-way through one group says so once more instead of standing on whatever it said first. The second line is service discovery's. With enableAudioSink false the media and speech channels are never declared, so the phone never opens them and never sends audio. Nothing said so anywhere. The log of a deliberately muted head unit is identical to the log of one whose audio has broken: no channel setup, no media messages, every audio instrument reading zero, and no cause in the file. It has already cost a test round, spent on an audio run that could not have produced audio, before the setting was found by reading the device's settings.xml rather than the capture. One INFO line on the else branch, following the pattern the Bluetooth service already uses eleven lines below, and worded so a reporter can act on it rather than in terms of the setting key.
…orks The dummy VPN was stopped from the last line of stopWirelessServer(), which initWifiMode() runs on every mode change. A andreknieriem#839 reporter described the VPN icon disappearing on their second connection and the audio stutters coming back with it; their log shows the mechanism outright: 03:28:00.278 AapService.initWifiMode | Initializing WiFi Mode: 3 03:28:00.281 VpnControl.stopVpn | Stopping DummyVpnService Three milliseconds. The wireless server never owned the VPN, so it stops deciding. DummyVpnPolicy holds the ownership rules: SELF_MODE or SESSION, and a VPN with no owner is never touched. Only the teardowns that own it call stopDummyVpn(), and there is deliberately no Reason for a wireless re-init. Removing that stop cost the one cleanup it did by accident, so startSelfMode() now arms a 120s watchdog: Self Mode with no phone takes the tun back down rather than leaving the unit without IPv4. A second reporter on different hardware saw the same periodic outage go quiet while the VPN happened to be up, which is what the new setting is for. It is off by default and gated twice. keepDummyVpnDuringSession is the trigger, and shouldStartForSession re-tests the connection mode at the point of use: the toggle only renders inside the Native AA block, so a preference turned on there and then hidden by a mode change would otherwise put a blackholing tun on a USB session. The mechanism it claims is a working theory, not a measurement, and the setting's own description says so. A session VPN also has to leave this app's own sockets alone, which Self Mode's never had to. An Android VPN captures its own app's traffic too, and this one routes 0.0.0.0/0 into a descriptor nobody reads, so every socket opened after it comes up is dropped, including the one a reconnect needs. The session owner therefore establishes with addDisallowedApplication(packageName); Self Mode still must not, because its whole purpose is a non-null activeNetwork for our own process. The flavor seam is the second half. Google does not allow the fake VPN this app uses for offline Self Mode, so the playstore flavor must ship none of it. isVpnAvailable() was the seam meant to say that, but the only copy of VpnControl hardcoded true and lived in app/src/github/, which is why the flavor could not compile at all. The reason nobody ever added the missing copy is one line of .gitignore: "playstore/", sitting next to "cert/" and clearly meant for a root-level store assets folder. The pattern is unanchored, so it matched app/src/playstore/ too and silently swallowed anything put there. !app/src/playstore/ re-includes it. app/src/playstore/.../VpnControl.kt now answers no to everything, and main stops touching android.net.VpnService and DummyVpnService directly: VpnService.prepare moves behind consentIntent() and isPrepared(), so Self Mode and the new session toggle go through the same seam. The prepared check is short-circuited on isVpnAvailable() so the playstore path cannot reach a prepare() call at all. The seam covers the copy as well as the code: the keep_dummy_vpn strings live in app/src/github/res/values/strings.xml, so the words are absent from the Play Store APK, and main reads them through VpnControl's resource ids, which the playstore copy answers 0. Both copies must keep identical signatures. The last part is the teardown, and it is the part that was never working at all. A rig round on an Android 14 unit ran the session VPN through two teardowns. Both of them logged exactly what they were supposed to: releasing the dummy VPN (owner=SESSION, reason=SESSION_ENDED) VpnControl: Stopping DummyVpnService (GitHub Build) and neither did anything. tun0 stayed <POINTOPOINT,UP,LOWER_UP> from the first establish() through the end of the round, the service's createTime never advanced, and dumpsys showed why: Client AppBindRecord{... system/1000}, hasBound=true. The framework binds to a VpnService the moment establish() succeeds, so stopService() clears the started flag and stops there. A bound service is not destroyed, onDestroy() never runs, and the descriptor stays open. Only killing the process took the tun down. The cost is the one the app announces when it brings the VPN up, that other apps on the unit have no IPv4, and it was being paid from the first session that raised the tun until the process died rather than for the session that asked for it. Self Mode's teardown goes through the same call, so this was never confined to the new setting. Asking the service to stop through an Intent is not the answer either: starting a non-foreground service from the background throws on O+, which is precisely where the teardown runs, since AapService.onDestroy() calls stopForeground(true) on the line before stopDummyVpn(). So the stop does not go through an Intent at all and closes the descriptor on the running instance directly. stopService() stays behind it for a service that was started but never established, where there is no instance and only the started flag to clear. onRevoke() was unhandled next door, with the same shape of silent failure: after the user or another VPN app takes the slot, vpnInterface stays non-null and startVpn()'s idempotence guard refuses to establish a new tun for the rest of the process's life. It now clears that state. Since a revoke arrives on a binder thread and the close can now come from any caller's thread, startVpn and stopVpn are synchronized.
Wireless bring-up is driven entirely by the stored wifiConnectionMode and never asked what the live session is riding on. initWifiMode() runs from onCreate() before USB is even probed, and onConnected() stopped nothing, so a wired session carried all of it alongside. A reporter's USB capture has every piece of it at once, on Native AA: 03:00:33 Native AA recovery (no phone joined within 60s): recreate attempt 2/4 03:00:33 Standard createGroup SUCCESS! group up 10s before USB connects 03:00:43 SSL handshake complete USB session live 03:00:47 Stopping poke retry loop (session=true) 4.1s after the handshake 03:01:35 Native AA join watchdog fired but a session is already connected 03:01:49 NearbyManager: Stopping discovery ... only at user exit 03:01:49 User exit with wirelessServer active :5288 bound throughout 03:01:51 WifiDirectManager: Stopping and cleaning up The P2P group, the RFCOMM listeners, Nearby, the :5288 server and the join watchdog all ran for the full session, and none of them can succeed while USB owns it: no phone can join a group it is not looking for. The watchdog only avoided tearing the group down mid-session because of a guard added for a different reason. The poke loop is the half with teeth. It does refuse to poke once a session exists, but it only asks at the top of each iteration and an iteration is a 15s hold plus a 15s gap, so 4.1s here and 30s worst case are spent opening RFCOMM connections into a live link. Each one raises an OS-level ACL_CONNECTED that AutoStartReceiver reads as the user's phone arriving, and the answer to that can be initWifiMode(force = true), which rebuilds the P2P group underneath a session that is already up. So: - onConnected() asks commManager.isWirelessSession, not the settings, and on a wired session stops NetworkDiscovery, Nearby, the Native AA handshake manager and the P2P group. Stopping NetworkDiscovery matters beyond the re-arm: the isBusy gate stops the next sweep, while the one in flight keeps probing 254 addresses on two ports each. - The poke loop is now cancelled on the connect event rather than polled, for every transport, via NativeAaHandshakeManager.onSessionEstablished(). - initWifiMode() refuses to arm while a wired session is live, so the Bluetooth auto-start path cannot walk the stack back up behind it. - No HIGH_PERF WiFi lock for a session whose bytes are not on the radio. - Disconnect re-arms with one initWifiMode(force = true), which subsumes the mode-3 reset and the user-exit teardown; running those as well would race a stop() against the re-arm's createGroup. Re-arming on a user exit too, because unplugging has to return the unit to its configured mode either way. Taking the group down here is safe precisely because no phone can be on it, and it is never reused: the re-arm creates a fresh one, so the never-reuse-a-P2P-group rule holds by construction. UsbSessionQuiescePolicy states the rule and the quiesce/re-arm symmetry, with tests. A stack taken down with no way back is how a long-lived manager ends up unable to re-arm.
…nects Most reports that call the hotspot transport dead are a head unit that could not read its own hotspot name and password, so the credentials handed to the phone were empty and nothing ever joined the network. The app already detected this exactly. SoftApCredentialsPolicy.decide returns a dedicated CONFIG_UNREADABLE verdict meaning "there is an access point and this device will not name it, and waiting cannot change that", the provider stops polling and logs the remedy, and the overrides that fix it have had settings rows all along. What was missing is reach, not detection: the verdict is produced during a handshake, delivered as a toast over the projection screen nobody is reading, and every string naming the remedy is phrased as "if the log says the name could not be read". The failure is reported to the audience least able to act on it, and the values that would fix it sit unset a few rows below. So the same question is now asked when the user picks Native mode or switches the transport, which is when they are already in Settings with a keyboard. NativeCredentialsPreflightPolicy is pure and answers per field: supplied by the user, available from the device, must be entered, or unknown. NativeCredentialsPreflight gathers the facts and decides nothing. Two properties keep it from becoming noise: Only certainties are reported. The hotspot name and password are answerable with the access point down, because getSoftApConfiguration reads the stored configuration and a refusal there is a permission refusal, so a no now is a no at handshake time. A MAC is not: it has to be read off an interface that exists, so it is reported inconclusive whenever there was nothing to read from, and unknown is never prompted for. The hotspot probe also refuses to answer while the framework says no access point is running, because pickApInterface will otherwise land on a cellular bridge and report confidently about the wrong network, which is the trap NativeCredentialsPolicy.shouldPublishCredentials already exists for. Nothing is asked for that could not work. The WiFi Direct route reports the BSSID and nothing else: its network name and passphrase are generated by the framework per group and read back off WifiP2pGroup, so a name the user typed could not match the group the phone is told to join, and no setting for one exists. Where location services are off, the toggle is offered instead of a MAC, because a static override outranks every automatic source afterwards and so outlives the problem it was entered for. No new settings rows. The check hangs off handleNativeAaSelection, which already ran a compatibility gate on mode 3 and showed a dialog, and the prompts reuse the same entry dialogs and the same copy as the rows themselves. Two fixes on the path this feeds, which it cannot ship without, since its whole purpose is to get more users hand-typing MAC addresses: WifiDirectManager took the static BSSID verbatim whenever it was anything other than the unset sentinel "0". A mistyped address won the chain, did not match the masked-string test below it, suppressed all six fallbacks, and surfaced 30 s later at Type 3 time as a message blaming location services. It now goes through SoftApBssidPolicy.choose, which is what the hotspot route has used since the same bug was found there, and the value is validated where it is typed rather than accepted and dealt with later. A P2P group that came up with no network name was dropped with no else branch and no log at all, reaching the reader only as the handshake's generic "No WiFi credentials available after Ns wait", which points at the credentials wait rather than at the group that never named itself. The runtime half keeps its toast and gains a notification behind it, on its own channel, with an id per condition and cleared as soon as that condition clears. The WiFi Direct BSSID abort, until now the loudest failure in the app with no user-visible signal whatsoever, raises one too. 17 unit tests pin the outcome table. Two cover a blanked hotspot configuration, which arrives as a present pair of empty strings because HotspotConfigReader's pre-Q branch returns the fields with no emptiness check. Testing nullness there asked only for the name, and setting a manual name is what makes SoftApCredentialsPolicy.resolve stop consulting the device, so the check would have walked the user into the open-network trap that policy exists to prevent. Not built or run: there is no Android toolchain on the authoring machine, so CI is the first compile of every line here.
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.
Why
Reporters on several units from #839 and #824 describe picture and sound cutting out together every ten seconds on a session that looks healthy in every line the app prints. A 2x2 of codec against whether the head unit's own WiFi station is joined elsewhere settles the variable: joined, no dropout over 0.65 s in 138 s and zero audio underruns; unjoined, four to eight seconds of black every ten and a half, thirty underruns in two minutes, both codecs alike.
Three instruments missed it.
LinkGapMonitormeasured all channels at once, and the phone's once-a-second CONTROL ping kept a link scoring healthy while both media channels were gone. Nothing measured the uplink, thoughmax_unackedflow control means stalled MediaAcks stop video and audio while leaving control alone. AndlogStationCoexistenceprinted only when the station was associated, so a missing line meant either "not joined" or "the read threw".The VPN and USB commits are here because they land in the same files. A reporter's log has the VPN mechanism outright:
Initializing WiFi Mode: 3at 03:28:00.278,Stopping DummyVpnServiceat 03:28:00.281, becausestopWirelessServer()ended by stopping a VPN it never owned and every mode change runs it. A separate wired capture shows the P2P group, RFCOMM listeners, Nearby, the :5288 server and the join watchdog all running for a full USB session, none of which can succeed while USB owns it.What changed
LinkGapMonitorruns per series (all-channel, video, three audio sinks) so a dead radio is separable from a stalled media path; newUplinkStallMonitormeasures the send side;StationCoexistencePolicykeeps its prescription only where two known frequencies establish it, sinceWifiP2pGroup.getFrequency()is API 29+ and older units reported zero and advised anyway;maybeRecoverFromDisplayStallno longer rebuilds projection and decoder mid-outage, which cost ~1.7 s more black plus a focus cycle for a fault no rebuild fixes.enableAudioSinkfalse a deliberately muted head unit logs identically to one whose audio has broken.DummyVpnPolicyholds ownership (SELF_MODE or SESSION), never touches an unowned VPN, and has no reason code for a wireless re-init. A new off-by-default setting keeps the VPN up for a Native AA session, gated twice. The playstore flavor gets theVpnControlcopy the seam always implied. Teardown closes the tun descriptor instead of asking the service to stop.onConnected()askscommManager.isWirelessSessionrather than the stored mode, and a wired session stops NetworkDiscovery, Nearby, the handshake manager and the P2P group; the poke loop is cancelled on the connect event;initWifiMode()refuses to arm while a wired session is live; no HIGH_PERF WiFi lock.UsbSessionQuiescePolicystates the quiesce and re-arm symmetry, with tests.Four new pure policy objects, all unit tested:
UplinkStallMonitor,StationCoexistencePolicy,UsbSessionQuiescePolicy,DummyVpnPolicy.LinkGapMonitorandProjectionWatchdogPolicygained tests alongside their changes. Neither monitor's report is read by anything yet; both print at INFO.Where to focus review
The VPN teardown, because both obvious implementations are wrong.
stopService()cannot tear down aVpnServicethat has calledestablish(): the framework binds at that moment, so the started flag clears andonDestroy()never runs. The previous code,startService()carrying a stop action, throws on O+ while the app is losing foreground, andAapService.onDestroy()callsstopForeground(true)on the line before. The stop now closes the descriptor on the running instance directly, withstopService()behind it only for a service started but never established.The USB commit takes a P2P group down on a wired connect. Safe because no phone can be on a group it is not looking for, and the group is never reused: the disconnect re-arm creates a fresh one.
The
.gitignorechange is load-bearing.playstore/sat unanchored next tocert/and matchedapp/src/playstore/, silently swallowing anything put there, which is why the flavor'sVpnControlcopy was never added.Verification
Built and unit tested on the rig twice:
assembleGithubDebugclean,testGithubDebugUnitTestgreen, 612/612 both times.DummyVpnPolicyTest7/7,UsbSessionQuiescePolicyTest8/8,LinkGapMonitorTest17/17,StationCoexistencePolicyTest11/11.Hardware: UNISOC MT50, Android 14, Native AA wireless, POCO X3 NFC, four rounds. Diagnostics: three minutes of a stationary Maps screen printed zero
inbound video quietlines at 39-46 fps, the false positive the recurrence floor was redesigned for (four windows at 95/99/96/99% dead against 55-60% for the real fault); a clean ten-minute session printed none of the new lines.Not covered
@andreknieriem please take a look if the Play Store exclusion on #f66ac7e28a5435c7e431cf0f11027c70705db8a5 for the VPN is correct