0da9e7d1757579b580cdb807475f74223b6e5fe5
2 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Yaojia Wang
|
b09b90e5bb |
fix(android): the ThumbnailPipeline bugs its skipped review would have caught
This file's cross-review was explicitly skipped when it was written — PROGRESS_ANDROID.md says so — and it had no production call sites until this session, so nothing had ever reviewed OR executed it. Wiring it to the home screen finally made an adversarial concurrency review worthwhile. It confirmed the hard parts are right (the check-then-insert dedup critical section, withPermit's exactly-once release with the fallback outside it, the NonCancellable finally, and rethrowing CancellationException) and then found five real defects. Every premise was re-verified from the termux v0.118.0 bytecode before fixing. 1. Thumbnails drew DUPLICATED ROWS. TerminalBuffer.externalToInternalRow is `(mScreenFirstRow + row) % mTotalRows` and mTotalRows is the constructor's transcriptRows, so passing TERMINAL_TRANSCRIPT_ROWS_MIN (100) while rows went up to 200 aliased external rows 100..199 onto 0..99 — and left the emulator's scroll state invalid during append(). A pure ThumbnailGrid now derives transcriptRows = max(rows, MIN). 2. A legitimately-sized session was LMK bait. cols/rows are server-supplied and the protocol validates resize only to [1,1000], so a 1000x1000 session forced 2800x2800 ARGB_8888 = 31.4 MB, twice over for two permits, every poll tick. Rather than clamp the grid — which would CROP a real screen — the cell size is now derived from the target, shrinking the font's own cell by an exact integer fraction with the aspect preserved. Any geometry in 1..1000 now yields at most 480x480 = 900 KiB; a real 161x50 goes from 3.1 MB to 322 KiB. 3. Two stalled fetches wedged the feature PERMANENTLY. There is no OkHttp callTimeout (the per-socket read timeout is reset by every trickled byte) and the render is deliberately uncancellable by any caller, so two slow-trickle previews held both permits until close() and no thumbnail rendered again. A withTimeout now sits inside withPermit, budgeting the work rather than the permit wait. The subtlety that makes this correct: TimeoutCancellationException IS a CancellationException, so it is caught BEFORE that branch and routed to the placeholder — otherwise a slow host would still kill the pipeline instead of degrading. 4. The in-flight entry was leakable, contradicting a comment that claimed it never was. `scope.async` on an already-cancelled scope never runs its body, so the finally never ran: a post-close() request left a dead Deferred that every later request awaited forever, and inFlight grew unbounded. Now guarded inside the mutex, and the comment names the one path that genuinely cannot release (where the map dies with the pipeline anyway). 5. Obsolete renders did guaranteed wasted work. lastOutputAt advances on every PTY byte and the list polls every 5s, so an active session mints a new key each tick; the row cancelled the old caller but the old render ran a full GET plus a full raster whose bitmap was cached under a key nothing would request again — occupying both permits ahead of the key actually on screen, and growing without bound once render latency exceeded arrival rate. A newer key now cancels the older in-flight render for that session. Also made two false statements true rather than leaving them: retainOnly now prunes under the same monitor publish() takes, so a render landing after the prune cannot resurrect a killed session's bitmap; and PreviewCap's KDoc no longer claims to prevent an oversized-body OOM (the body is already buffered and parsed by then) nor to "mirror the server's cap" (the server defaults to 24 KiB, not 256 KiB, and an operator can raise it unbounded). RED was observed for all four behavioural fixes before implementing. Verified: ./gradlew test :app:testDebugUnitTest :app:assembleDebug koverVerify -> BUILD SUCCESSFUL, 901 JVM tests, 0 failures (893 -> 901). The agent distrusted a FROM-CACHE green, forced a real execution, and confirmed the code reached the packaged APK by scanning its dex. |
||
|
Yaojia Wang
|
4ea8f7862a |
feat(android): pure-Kotlin foundation — AW0 scaffold + AW1 modules (218 tests green)
Implements the verifiable pure-Kotlin core of the Android client per
docs/ANDROID_CLIENT_PLAN.md, mirroring the iOS SPM package set as Gradle modules.
Built + reviewed via multi-agent workflow (explore→implement→verify→review),
then review findings fixed with regression tests.
Modules (all pure JVM, kotlin("jvm"); Android-framework modules scaffolded but
gated off in settings — no Android SDK in this env):
- :wire-protocol — frozen wire contract (sealed Client/ServerMessage, enums,
HostEndpoint CSWSH origin derivation, transport interfaces), hand-rolled codec
byte-identical to the server's JSON.stringify + tolerant kotlinx decode.
- :session-core — ReconnectMachine (1→2→4→8→16→30 backoff), PingScheduler,
GateTracker (two-line epoch guard), AwayDigest, UnreadLedger, TitleSanitizer,
KeyByteMap (byte-matches public/keybar.ts).
- :api-client — all REST routes, tolerant decode, Origin-iff-guarded, prefs
unknown-key preservation, pairing probe + host-tier classifier.
- :client-tls — pure half: PKCS#12 parse, X509KeyManager alias logic,
CertificateSummary, provider-agnostic wrong-passphrase classification.
- :test-support — FakeTransport / FakeHttpTransport / virtual-clock fakes.
Verify: ./gradlew clean test → 218 tests, 0 failures (wire-protocol 47,
session-core 60, api-client 69, client-tls 27, test-support 15).
Review (3 lenses, 8/10 each) findings fixed: sessionId JSON-injection escape,
CancellationException propagation in PingScheduler, PairingProbe close-on-cancel
leak, HostEndpoint trim, HostClassifier hoisted to :wire-protocol (type unified),
BouncyCastle-portable wrong-passphrase detection, lone-surrogate escaping.
Known gap (documented, deferred): /push/fcm-token has no server route yet —
plan task A33 (src/push/fcm.ts) delivers it.
|