fix(android): move emulator mutation to the render thread

The instrumented suite was run on a device for the first time and the app died
on the first test:

    ArrayIndexOutOfBoundsException: length=31; index=31
        at com.termux.terminal.TerminalRow.getStyle
        at com.termux.view.TerminalRenderer.render
        at com.termux.view.TerminalView.onDraw

PROGRESS_ANDROID.md had classified exactly this as "Accepted (not a defect):
steady-state append runs concurrently with the UI-thread onDraw ... torn read
self-corrects next frame". Both halves of that were wrong.

It is not cosmetic. TerminalRenderer.render caches its column bound once from
TerminalEmulator.mColumns and then indexes TerminalRow.getStyle(column) into a
raw long[] sized when that row was allocated. TerminalEmulator.resize publishes
the new mColumns BEFORE resizeScreen() reallocates the rows, so a draw landing
in that window reads the new bound against old-width rows. The first
out-of-range column is exactly oldColumns, which is why the exception was
always length == index — the consistency was a deterministic signature of a
grow, not a rare interleaving. A shrink is harmless, which is probably why
"self-corrects" looked plausible. TerminalBuffer.resize is non-atomic in
several more ways, and TerminalRow.setChar swapping mText for a larger array is
the same hazard on the append path.

Nor does it match upstream. Upstream's background reader only fills a
ByteQueue; TerminalSession$MainThreadHandler is what calls append, and
TerminalView.updateSize resizes — both on the main thread. Upstream has no
concurrent reader at all. There is also no lock to lean on: monitorenter
appears nowhere in TerminalEmulator, TerminalBuffer, TerminalRow or
TerminalRenderer.

The race was dormant until this session. updateSize had no call sites, so
mColumns never changed after bind and there was never a width mismatch to tear
on. Making resize work is what made this reachable — the same lesson as the
rest of this pass: the green suite could not see any of it because no JVM test
instantiates a View.

Fix: every emulator mutation now runs on the render thread, from inside the
same single confined consumer. §6.2 is intact — still one writer draining one
ordered channel, suspending across the main hop, so a resize and an append
still cannot interleave and submission order still holds. They are now also
serialised against onDraw, because one Handler work item cannot run inside
another. Resize still reaches the wire and the §6.4 forced re-assert still
bypasses the dedup.

Chunking becomes load-bearing rather than incidental: each 4 KiB slice is its
own main-thread work item, so a multi-MB ring replay interleaves with frames
and input instead of blocking behind one long call — which is what §6.2's
no-ANR requirement actually needs, rather than "runs off the main thread".

Gating the child's dispatchDraw was considered and rejected: it would have to
cover appends too, leaving a choice between blanking the terminal and blocking
the UI thread on a full reflow. A read/write lock was rejected because append
calls back out through TerminalOutput (title, bell, DA/DSR reply), so holding a
write lock across it would bury a deadlock invariant in app-level callbacks.

The KDoc that made the false claims now states the mechanism with offsets, and
RemoteTerminalHostView records why dispatchDraw is deliberately not gated.

Verified: the device crash is reproduced as a JVM test first (red), so this
regression is now guarded without a device. ./gradlew test :app:assembleDebug
koverVerify -> 903 tests, 0 failures. On the emulator the previously-crashing
test now passes and 5 of 6 alternate-screen scroll tests pass.
This commit is contained in:
Yaojia Wang
2026-07-30 13:25:42 +02:00
parent c1612d0145
commit 538c8ebf34
3 changed files with 339 additions and 44 deletions

View File

@@ -76,6 +76,21 @@ import kotlin.math.roundToInt
* binding our emulator would have thrown. [init] sets the text size, which both creates the renderer and * binding our emulator would have thrown. [init] sets the text size, which both creates the renderer and
* is what makes the real cell metrics ([cellMetrics]) available for the resize path. * is what makes the real cell metrics ([cellMetrics]) available for the resize path.
* *
* ### Why `dispatchDraw` is NOT gated (the fourth 2026-07-30 crash, and where it was actually fixed)
* The stock `onDraw` → `TerminalRenderer.render` walk is not defensive: it caches its column bound from
* `TerminalEmulator.mColumns` and then indexes each row's `long[]` style array, which is sized to whatever
* `TerminalBuffer.mColumns` was when that row was allocated. A regrid landing mid-frame therefore killed
* the process with `ArrayIndexOutOfBoundsException: length=31; index=31`, and there is no lock to take —
* `monitorenter` appears nowhere in the pinned emulator or renderer. This frame owns the child's
* `dispatchDraw`, so suppressing the draw during a mutation was one available fix; it was NOT the one
* chosen, because it would have to cover every mutation (appends swap `TerminalRow.mText` too) and would
* mean either blanking the terminal or blocking the UI thread on a 10 000-row reflow.
*
* Instead [RemoteTerminalSession] mutates the emulator **on this thread** — the main thread — exactly as
* upstream Termux does, so a mutation and a draw are two main-thread work items and cannot interleave at
* all. That is the invariant this view depends on: **nothing may mutate `stockView.mEmulator`'s state off
* the main thread.** If that is ever broken, gating here becomes necessary again.
*
* DEVICE QA (plan §7): focus/IME behaviour across IMEs, CJK composition, link taps, scroll feel (slop, * DEVICE QA (plan §7): focus/IME behaviour across IMEs, CJK composition, link taps, scroll feel (slop,
* momentum) and the real font-metric grid are verified on a device — this class is glue, and the logic it * momentum) and the real font-metric grid are verified on a device — this class is glue, and the logic it
* delegates to ([TerminalKeyDecoder], [TerminalInputBytes], [TerminalResizeDriver], [TerminalScrollGesture]) * delegates to ([TerminalKeyDecoder], [TerminalInputBytes], [TerminalResizeDriver], [TerminalScrollGesture])

View File

@@ -14,7 +14,6 @@ import kotlinx.coroutines.cancel
import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.channels.Channel
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.withContext import kotlinx.coroutines.withContext
import kotlinx.coroutines.yield
import java.util.ArrayDeque import java.util.ArrayDeque
import java.util.concurrent.Executors import java.util.concurrent.Executors
import wang.yaojia.webterm.wire.ClientMessage import wang.yaojia.webterm.wire.ClientMessage
@@ -30,29 +29,62 @@ import wang.yaojia.webterm.wire.ClientMessage
* `:wire-protocol` ([ClientMessage]) — the `SessionEvent → ByteArray` decode is A21's, not here * `:wire-protocol` ([ClientMessage]) — the `SessionEvent → ByteArray` decode is A21's, not here
* (boundary note §3). * (boundary note §3).
* *
* ### Single-writer confinement (plan §6.2 mustFix) * ### Single-writer confinement, and where the writer actually runs (plan §6.2 — CORRECTED 2026-07-30)
* `TerminalEmulator`/`TerminalBuffer` is single-writer, read by the renderer on the UI thread. Every * There is exactly ONE writer: an ordered command [Channel] drained by a single consumer on
* mutation (append, resize, pending flush) is serialized onto ONE confined dispatcher via an ordered * [confinedDispatcher], so every mutation (append, resize, pending flush) happens in strict submission
* command [Channel] — mirroring Termux's background reader thread — so a multi-MB ring replay never * order and never concurrently with another mutation. What CHANGED is where the mutation itself executes:
* blocks the UI thread (ANR) and never races the on-screen draw. Only [onScreenUpdated] is posted to * the consumer hops to [mainDispatcher] for the emulator call and nothing else.
* [mainDispatcher]. Ordering is exactly submission order (FIFO channel + one consumer). *
* **Why it has to.** The stock renderer reads [TerminalBuffer] on the UI thread during `onDraw`, and the
* pinned Termux artifacts contain **no synchronisation whatsoever** — `monitorenter` appears nowhere in
* `TerminalEmulator`, `TerminalBuffer`, `TerminalRow` or `TerminalRenderer`. Upstream Termux is safe not
* by locking but by **thread identity**: its background reader only fills a `ByteQueue`, and
* `TerminalSession$MainThreadHandler.handleMessage` is what calls `TerminalEmulator.append` — on the main
* thread — while `TerminalView.updateSize()` resizes from the main thread too. So upstream has no
* concurrent reader at all, and this fork's earlier "single writer racing the UI-thread renderer, torn
* reads self-correct next frame" note was wrong twice over: it did not match upstream, and the tear is
* not cosmetic. `TerminalRenderer.render` caches its column bound from `TerminalEmulator.mColumns`
* (offsets 14-18) and then indexes `TerminalRow.getStyle(column)` — a raw `long[]` sized when the row was
* allocated — while `TerminalEmulator.resize` publishes the new `mColumns` (offsets 97-106) BEFORE
* `resizeScreen()` reallocates the rows (offset 160). A draw landing in that window throws
* `ArrayIndexOutOfBoundsException: length=oldColumns; index=oldColumns` on the UI thread: a process kill,
* observed on a real emulator. `TerminalBuffer.resize` is non-atomic in five more ways besides (it
* republishes `mLines`, then `mTotalRows`/`mScreenRows`/`mScreenFirstRow`/`mActiveTranscriptRows`), one of
* which fails as `IllegalArgumentException` out of `externalToInternalRow`; and `TerminalRow.setChar`
* swaps `mText` for a larger array, which is the same hazard on the append path.
*
* **Why this still honours §6.2's mustFix (no ANR on a multi-MB replay).** §6.2 forbids a *synchronous
* multi-MB* append on the main thread. The append is still [APPEND_CHUNK_BYTES]-chunked, and each chunk is
* its OWN main-thread work item, so the looper regains control between chunks: input and frames interleave
* with the replay instead of waiting behind it. No single work item is long, which is what "does not ANR"
* actually requires — not "runs on another thread".
*
* **Deadlock safety.** [confinedDispatcher] is never held while waiting on anything except the main hop
* itself, and the main thread never waits on the writer (there is no lock in either direction). The
* consumer is the only caller of the emulator, so an emulator callback re-entering this class (title,
* bell, DA/DSR reply via [termOutput]) runs on the main thread inside the hop and must therefore stay
* non-blocking — [engineSend] is documented as safe to call from any thread and must not block.
* *
* The emulator is **not** exposed to the stock renderer until the initial [pendingOutput] flush has * The emulator is **not** exposed to the stock renderer until the initial [pendingOutput] flush has
* COMPLETED: the Bind command flushes the queue on the confined thread first, THEN posts a single Main * COMPLETED: the Bind command flushes the queue first, THEN publishes `mEmulator` to the view and fires
* action that publishes `mEmulator` to the view AND fires the first screen update — so the renderer * the first screen update, so the renderer never sees a half-replayed buffer at the bind moment.
* never reads [TerminalBuffer] while the confined thread is still appending at the bind moment (§6.2).
* One malformed/hostile escape byte cannot freeze the terminal either: [consumeCommands] survives a * One malformed/hostile escape byte cannot freeze the terminal either: [consumeCommands] survives a
* per-command throw and keeps draining (only cancellation propagates). * per-command throw and keeps draining (only cancellation propagates).
* *
* @param engineSend the outbound sink — every [ClientMessage] produced here (Input/Resize) is handed to * @param engineSend the outbound sink — every [ClientMessage] produced here (Input/Resize) is handed to
* it. A21 bridges it to the engine's ordered send pump (§6.3). Must be safe to call from any thread. * it. A21 bridges it to the engine's ordered send pump (§6.3). Must be safe to call from any thread,
* and must not block: it can be invoked from inside an emulator callback on the main thread.
* @param onTitleChanged raw OSC 0/2 title delegate. Passed through UNsanitized — :terminal-view has no * @param onTitleChanged raw OSC 0/2 title delegate. Passed through UNsanitized — :terminal-view has no
* `:session-core` edge, so :app wires this to `TitleSanitizer` (plan §6.5 / boundary note §3). * `:session-core` edge, so :app wires this to `TitleSanitizer` (plan §6.5 / boundary note §3). Called on
* @param onBell OSC/ctrl-G bell delegate (the view may buzz/flash). * [mainDispatcher] from inside `append`; must not block.
* @param appendDispatcher the confined single-writer dispatcher (default: a private single-thread * @param onBell OSC/ctrl-G bell delegate (the view may buzz/flash). Same threading note as
* executor). Injected as a `StandardTestDispatcher` in unit tests so the seam drives under virtual time. * [onTitleChanged].
* @param mainDispatcher where [onScreenUpdated] is posted (default `Main.immediate`; a test dispatcher * @param appendDispatcher the confined dispatcher that SEQUENCES commands and slices appends (default: a
* in unit tests so no real main looper is needed). * private single-thread executor). It no longer touches emulator state — [mainDispatcher] does. Injected
* as a `StandardTestDispatcher` in unit tests so the seam drives under virtual time.
* @param mainDispatcher the renderer's own thread: where every emulator mutation and every
* [onScreenUpdated] runs (default `Main.immediate`; a test dispatcher in unit tests so no real main
* looper is needed).
*/ */
public class RemoteTerminalSession( public class RemoteTerminalSession(
private val engineSend: (ClientMessage) -> Unit, private val engineSend: (ClientMessage) -> Unit,
@@ -64,7 +96,10 @@ public class RemoteTerminalSession(
appendDispatcher: CoroutineDispatcher? = null, appendDispatcher: CoroutineDispatcher? = null,
private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main.immediate, private val mainDispatcher: CoroutineDispatcher = Dispatchers.Main.immediate,
) { ) {
/** Emulator-originated output sink + title/clipboard/bell delegate. Runs on the confined thread. */ /**
* Emulator-originated output sink + title/clipboard/bell delegate. Every method here is called from
* inside `TerminalEmulator.append`, i.e. on [mainDispatcher] — so none of them may block.
*/
private val termOutput: TerminalOutput = object : TerminalOutput() { private val termOutput: TerminalOutput = object : TerminalOutput() {
override fun write(data: ByteArray?, offset: Int, count: Int) { override fun write(data: ByteArray?, offset: Int, count: Int) {
if (data == null || count <= 0) return if (data == null || count <= 0) return
@@ -89,7 +124,7 @@ public class RemoteTerminalSession(
public val emulator: TerminalEmulator = public val emulator: TerminalEmulator =
TerminalEmulator(termOutput, initialCols, initialRows, transcriptRows, NoOpTerminalSessionClient()) TerminalEmulator(termOutput, initialCols, initialRows, transcriptRows, NoOpTerminalSessionClient())
// ── Confinement plumbing (all mutable state below is touched ONLY on the confined thread) ─────── // ── Sequencing plumbing (all mutable state below is touched ONLY on the confined consumer) ───────
private val ownedExecutor = if (appendDispatcher == null) { private val ownedExecutor = if (appendDispatcher == null) {
Executors.newSingleThreadExecutor { r -> Thread(r, "webterm-term-append").apply { isDaemon = true } } Executors.newSingleThreadExecutor { r -> Thread(r, "webterm-term-append").apply { isDaemon = true } }
} else { } else {
@@ -115,7 +150,7 @@ public class RemoteTerminalSession(
/** /**
* Feed remote WS output into the emulator. Never blocks the caller (the command channel is * Feed remote WS output into the emulator. Never blocks the caller (the command channel is
* UNLIMITED) and never mutates the emulator on the caller's thread — the bytes are appended on the * UNLIMITED) and never mutates the emulator on the caller's thread — the bytes are appended on the
* confined dispatcher. Output arriving before [attachView]/[bind] is queued and replayed in order * renderer's own thread, chunked. Output arriving before [attachView]/[bind] is queued and replayed in order
* (§6.2); the `ESC[0m` replay prefix is passed through, never stripped. * (§6.2); the `ESC[0m` replay prefix is passed through, never stripped.
*/ */
public fun feedRemote(bytes: ByteArray) { public fun feedRemote(bytes: ByteArray) {
@@ -126,8 +161,8 @@ public class RemoteTerminalSession(
/** /**
* Latest-writer-wins resize (§6.4). Resolves the emulator's local buffer reflow AND emits a * Latest-writer-wins resize (§6.4). Resolves the emulator's local buffer reflow AND emits a
* `Resize` to the server (→ SIGWINCH) — normally only when the dims actually change from * `Resize` to the server (→ SIGWINCH) — normally only when the dims actually change from
* [lastSentDims] and are positive. The dedup + `emulator.resize` are serialized on the confined * [lastSentDims] and are positive. The dedup runs on the confined consumer so a resize never races an
* thread so a resize never races an append. * append; the reflow itself runs on [mainDispatcher] so it never races a draw either (`onResize`).
* *
* @param force emit even when the dims are unchanged. This is what makes the §6.4 reclaim work: a * @param force emit even when the dims are unchanged. This is what makes the §6.4 reclaim work: a
* shared PTY has ONE size, so a device coming back to the foreground has to re-assert dims it has * shared PTY has ONE size, so a device coming back to the foreground has to re-assert dims it has
@@ -165,11 +200,11 @@ public class RemoteTerminalSession(
// ── View binding (pendingOutput flush) ─────────────────────────────────────────────────────────── // ── View binding (pendingOutput flush) ───────────────────────────────────────────────────────────
/** /**
* Bind the on-screen view. The emulator is NOT published to the stock renderer synchronously here * Bind the on-screen view. The emulator is NOT published to the stock renderer synchronously here:
* (on Main): the confined thread may still be flushing [pendingOutput] into the buffer, and the * [pendingOutput] may still hold an unflushed ring replay, and a renderer that can see the buffer
* renderer reads it during draw — publishing now would race that flush (§6.2). Instead the publish * mid-replay paints a half-restored screen. So the publish is routed through the command channel and
* is routed through the command channel so it lands on [mainDispatcher] only AFTER the initial flush * lands only AFTER the initial flush completes, together with the first screen update. The sink then
* completes, together with the first screen update. The sink then invalidates the view on every feed. * invalidates the view on every feed.
*/ */
public fun attachView(view: RemoteTerminalView) { public fun attachView(view: RemoteTerminalView) {
view.session = this view.session = this
@@ -182,10 +217,10 @@ public class RemoteTerminalSession(
} }
/** /**
* Testable bind seam: register the [onScreenUpdated] sink, flush pending output on the confined * Testable bind seam: register the [onScreenUpdated] sink, flush pending output, THEN publish the
* thread, THEN publish the emulator ([publishEmulator]) + fire the first screen update on the main * emulator ([publishEmulator]) + fire the first screen update. Production goes through [attachView]
* dispatcher. Production goes through [attachView] (which supplies [publishEmulator]); unit tests * (which supplies [publishEmulator]); unit tests call this directly (no android View needed) and
* call this directly (no android View needed) and default [publishEmulator] to a no-op. * default [publishEmulator] to a no-op.
*/ */
internal fun bind(publishEmulator: () -> Unit = {}, onScreenUpdated: () -> Unit) { internal fun bind(publishEmulator: () -> Unit = {}, onScreenUpdated: () -> Unit) {
commands.trySend(TerminalCommand.Bind(publishEmulator, onScreenUpdated)) commands.trySend(TerminalCommand.Bind(publishEmulator, onScreenUpdated))
@@ -198,7 +233,7 @@ public class RemoteTerminalSession(
ownedExecutor?.shutdownNow() ownedExecutor?.shutdownNow()
} }
// ── The single confined consumer (one writer to the emulator) ──────────────────────────────────── // ── The single confined consumer (the one writer; it mutates on [mainDispatcher]) ─────────────────
private suspend fun consumeCommands() { private suspend fun consumeCommands() {
for (command in commands) { for (command in commands) {
@@ -239,10 +274,9 @@ public class RemoteTerminalSession(
while (pendingOutput.isNotEmpty()) { while (pendingOutput.isNotEmpty()) {
appendChunked(pendingOutput.removeFirst()) appendChunked(pendingOutput.removeFirst())
} }
// The flush is now COMPLETE on this confined thread. Only now do we hand the emulator to the // The flush is now COMPLETE. Only now do we hand the emulator to the stock renderer AND fire the
// stock renderer AND fire the first screen update — both on the main dispatcher, in one action // first screen update — one main-thread action ordered strictly after the last append, so the
// ordered strictly after the flush. The renderer therefore never reads TerminalBuffer while the // renderer's first frame sees the whole replay rather than a partially restored screen.
// confined thread is still appending at the bind moment (§6.2 single-writer-vs-UI-read).
withContext(mainDispatcher) { withContext(mainDispatcher) {
publishEmulator() publishEmulator()
onScreenUpdated() onScreenUpdated()
@@ -254,17 +288,25 @@ public class RemoteTerminalSession(
// `force` is the §6.4 reclaim: unchanged dims must still reach the wire to win the shared PTY // `force` is the §6.4 reclaim: unchanged dims must still reach the wire to win the shared PTY
// size back. Everything else is deduped here as the last line of defence against layout churn. // size back. Everything else is deduped here as the last line of defence against layout churn.
if (next == lastSentDims && !force) return if (next == lastSentDims && !force) return
emulator.resize(cols, rows) // local buffer reflow (confined-thread single writer) // The reflow rewrote the on-screen cells, so the repaint rides along in the SAME main-thread work
// item as the mutation: no frame can be drawn between them, and without a repaint the user keeps
// looking at the old wrap points until the next byte arrives (on an idle Claude session, minutes).
val notifyScreenUpdated = screenUpdateSink
withContext(mainDispatcher) {
emulator.resize(cols, rows) // local buffer reflow — on the renderer's own thread
notifyScreenUpdated?.invoke()
}
engineSend(ClientMessage.Resize(cols, rows)) // → server SIGWINCH engineSend(ClientMessage.Resize(cols, rows)) // → server SIGWINCH
lastSentDims = next lastSentDims = next
// The reflow rewrote the on-screen cells; without a repaint the user keeps looking at the old
// wrap points until the next byte arrives (which, on an idle Claude session, can be minutes).
postScreenUpdate()
} }
/** /**
* Append [bytes] to the emulator in [APPEND_CHUNK_BYTES] slices, yielding between chunks so a * Append [bytes] to the emulator in [APPEND_CHUNK_BYTES] slices, one main-thread work item per slice.
* multi-MB replay stays cooperative (cancellable, and never monopolizes the confined thread). *
* The slicing is what keeps a multi-MB ring replay off the ANR path (§6.2): the main looper regains
* control between chunks, so frames and input interleave with the replay rather than queueing behind
* it. It also keeps the loop cooperatively cancellable — `withContext` is a suspension point.
*
* UTF-8 continuation state is buffered inside the emulator across `append` calls, so slicing at an * UTF-8 continuation state is buffered inside the emulator across `append` calls, so slicing at an
* arbitrary byte boundary never corrupts a multi-byte character. * arbitrary byte boundary never corrupts a multi-byte character.
*/ */
@@ -273,9 +315,8 @@ public class RemoteTerminalSession(
while (offset < bytes.size) { while (offset < bytes.size) {
val end = minOf(offset + APPEND_CHUNK_BYTES, bytes.size) val end = minOf(offset + APPEND_CHUNK_BYTES, bytes.size)
val chunk = if (offset == 0 && end == bytes.size) bytes else bytes.copyOfRange(offset, end) val chunk = if (offset == 0 && end == bytes.size) bytes else bytes.copyOfRange(offset, end)
emulator.append(chunk, chunk.size) withContext(mainDispatcher) { emulator.append(chunk, chunk.size) }
offset = end offset = end
if (offset < bytes.size) yield()
} }
} }

View File

@@ -0,0 +1,239 @@
package wang.yaojia.webterm.terminalview
import com.termux.terminal.TerminalEmulator
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Job
import kotlinx.coroutines.asCoroutineDispatcher
import kotlinx.coroutines.launch
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.yield
import org.junit.jupiter.api.Assertions.assertEquals
import org.junit.jupiter.api.Assertions.assertNull
import org.junit.jupiter.api.Assertions.assertTrue
import org.junit.jupiter.api.Test
import wang.yaojia.webterm.wire.ClientMessage
import java.util.concurrent.Executors
import java.util.concurrent.TimeUnit
import java.util.concurrent.atomic.AtomicBoolean
import java.util.concurrent.atomic.AtomicInteger
import java.util.concurrent.atomic.AtomicReference
/**
* REGRESSION for the fourth 2026-07-30 crash: a regrid on the confined writer thread tearing the
* `TerminalBuffer` out from under the stock renderer's UI-thread read.
*
* ## What was observed on a device (AVD `webterm`, android-35)
* ```
* FATAL EXCEPTION: main
* java.lang.ArrayIndexOutOfBoundsException: length=31; index=31
* at com.termux.terminal.TerminalRow.getStyle(TerminalRow.java:244)
* at com.termux.view.TerminalRenderer.render(TerminalRenderer.java:104)
* at com.termux.view.TerminalView.onDraw(TerminalView.java:921)
* ```
*
* ## Why `length == index` is a signature, not a coincidence
* `TerminalRenderer.render` caches its column bound ONCE, from `TerminalEmulator.mColumns`
* (`terminal-view-v0.118.0.aar`, `render` offsets 14-18), then walks `0 until columns` calling
* `TerminalRow.getStyle(column)` (offset 346) — a raw `long[]` index whose array was sized to the
* `TerminalBuffer.mColumns` in force when the row was allocated (`TerminalRow.<init>` offsets 21-25:
* `mStyle = new long[columns]`). `TerminalEmulator.resize` publishes its own `mColumns` FIRST (offsets
* 97-106) and only then calls `resizeScreen()` → `TerminalBuffer.resize` (offset 160), which is where the
* rows are actually reallocated. So a reader that starts between those two points sees the NEW bound
* against OLD rows, and the very first out-of-range column is exactly `oldColumns` — hence
* `index == length`, every time, for any grow.
*
* ## Why these two tests can live on the JVM
* `TerminalEmulator`/`TerminalBuffer`/`TerminalRow` are pure Java. [renderLikeRead] reproduces the
* renderer's *indexing contract* — cache the bound, then index rows — so the race is reachable without a
* `Canvas`, a `View` or a device. `TerminalResizeDrawRaceRegressionTest` (instrumented) covers the same
* defect through the real `onDraw`; this pair is the fast gate that keeps it from coming back.
*/
class EmulatorMutationThreadTest {
/**
* The crash itself: hammer alternating grids through [RemoteTerminalSession.updateSize] while a
* reader on the *render* thread walks the buffer the way `TerminalRenderer.render` does.
*
* Before the fix this throws within a handful of iterations (`AIOOBE length=31; index=31` from
* `getStyle`, or `IllegalArgumentException extRow=…, mScreenRows=…` from `externalToInternalRow` —
* the same regrid observed at a different offset). After it, the mutation and the read are both main
* -thread work items and can no longer interleave at all.
*
* The reader deliberately does NOT touch `TerminalRow.mText`: that array is reallocated by
* `setChar` (offsets 283-320 / 430-471) so it has its own, much rarer, tearing shape which would make
* this test flaky for a reason that is not the regrid. This test is about the regrid.
*/
@Test
fun `a regrid never tears the buffer out from under a reader on the render thread`() {
val renderThread = Executors.newSingleThreadExecutor { r -> Thread(r, RENDER_THREAD) }
val writerThread = Executors.newSingleThreadExecutor { r -> Thread(r, WRITER_THREAD) }
val resizesOnTheWire = AtomicInteger(0)
val session = RemoteTerminalSession(
engineSend = { if (it is ClientMessage.Resize) resizesOnTheWire.incrementAndGet() },
transcriptRows = TEST_TRANSCRIPT_ROWS,
appendDispatcher = writerThread.asCoroutineDispatcher(),
mainDispatcher = renderThread.asCoroutineDispatcher(),
)
session.bind {}
val failure = AtomicReference<Throwable?>(null)
val readsCompleted = AtomicInteger(0)
val stop = AtomicBoolean(false)
val reader = readerOnRenderThread(renderThread, session.emulator, stop, failure, readsCompleted)
try {
repeat(REGRID_ITERATIONS) { iteration ->
val cols = if (iteration % 2 == 0) NARROW_COLS else WIDE_COLS
session.updateSize(cols = cols, rows = ROWS)
session.feedRemote(SAMPLE_OUTPUT)
}
awaitTrue("all $REGRID_ITERATIONS regrids to reach the wire") {
resizesOnTheWire.get() >= REGRID_ITERATIONS || failure.get() != null
}
} finally {
stop.set(true)
runBlocking { reader.join() }
session.close()
renderThread.shutdownNow()
writerThread.shutdownNow()
}
assertNull(
failure.get(),
"the render thread saw a half-applied regrid — TerminalBuffer was mutated off the render " +
"thread (${failure.get()})",
)
assertTrue(
readsCompleted.get() > 0,
"the reader never ran, so this test proved nothing (completed=${readsCompleted.get()})",
)
assertEquals(
REGRID_ITERATIONS,
resizesOnTheWire.get(),
"every real grid change must still reach the server — the fix must not disable resize",
)
}
/**
* The invariant behind the fix, pinned positively: the emulator is mutated on the RENDER thread.
*
* `TerminalEmulator.append` calls `TerminalOutput.titleChanged` synchronously while parsing an
* `OSC 0 ; … BEL`, so the title delegate is a free probe for "which thread is inside `append`". If a
* future edit moves the append back onto the confined writer, this fails immediately and by name,
* rather than as a one-in-N crash on somebody's phone.
*/
@Test
fun `emulator mutation runs on the render thread, not the confined writer`() {
val renderThread = Executors.newSingleThreadExecutor { r -> Thread(r, RENDER_THREAD) }
val writerThread = Executors.newSingleThreadExecutor { r -> Thread(r, WRITER_THREAD) }
val threadInsideAppend = AtomicReference<String?>(null)
val session = RemoteTerminalSession(
engineSend = {},
onTitleChanged = { threadInsideAppend.set(hostThreadName()) },
appendDispatcher = writerThread.asCoroutineDispatcher(),
mainDispatcher = renderThread.asCoroutineDispatcher(),
)
session.bind {}
try {
session.feedRemote(TITLE_SEQUENCE)
awaitTrue("the OSC title to be parsed") { threadInsideAppend.get() != null }
} finally {
session.close()
renderThread.shutdownNow()
writerThread.shutdownNow()
}
assertEquals(
RENDER_THREAD,
threadInsideAppend.get(),
"TerminalEmulator.append must run on the same thread that draws it (stock TerminalRenderer " +
"reads TerminalBuffer with no synchronisation of any kind — `monitorenter` appears " +
"nowhere in the pinned emulator or renderer)",
)
}
// ── Helpers ──────────────────────────────────────────────────────────────────────────────────────
/**
* `TerminalRenderer.render`'s read contract, reduced to the indexing that can throw: cache the
* column/row bounds from the emulator (offsets 6-18), resolve each screen row through
* `externalToInternalRow` + `allocateFullLineIfNecessary` (offsets 173-185), then index the row's
* style array per column (offset 346).
*/
private fun renderLikeRead(emulator: TerminalEmulator) {
val columns = emulator.mColumns
val rows = emulator.mRows
val screen = emulator.screen
for (row in 0 until rows) {
val line = screen.allocateFullLineIfNecessary(screen.externalToInternalRow(row))
for (column in 0 until columns) {
line.getStyle(column)
}
}
}
private fun readerOnRenderThread(
renderThread: java.util.concurrent.ExecutorService,
emulator: TerminalEmulator,
stop: AtomicBoolean,
failure: AtomicReference<Throwable?>,
readsCompleted: AtomicInteger,
): Job = CoroutineScope(renderThread.asCoroutineDispatcher()).launch {
while (!stop.get()) {
try {
renderLikeRead(emulator)
readsCompleted.incrementAndGet()
} catch (t: Throwable) {
failure.compareAndSet(null, t)
return@launch
}
// Single-threaded dispatcher: yielding is what lets a queued mutation run at all. Without it
// this loop would monopolise the render thread and the test would pass vacuously.
yield()
}
}
/**
* The executor thread's own name, with the coroutines debug-agent's ` @coroutine#N` suffix stripped —
* that suffix is appended per-continuation, so the raw name is not a stable identity.
*/
private fun hostThreadName(): String = Thread.currentThread().name.substringBefore(COROUTINE_SUFFIX)
private fun awaitTrue(what: String, predicate: () -> Boolean) {
val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(AWAIT_TIMEOUT_SECONDS)
while (System.nanoTime() < deadline) {
if (predicate()) return
Thread.sleep(POLL_INTERVAL_MS)
}
assertTrue(predicate(), "timed out after $AWAIT_TIMEOUT_SECONDS s waiting for: $what")
}
private companion object {
const val RENDER_THREAD = "test-render-thread"
const val WRITER_THREAD = "test-writer-thread"
/** What `kotlinx-coroutines-debug` appends to a thread name while a continuation runs on it. */
const val COROUTINE_SUFFIX = " @coroutine#"
/** The observed crash was `length=31; index=31`; alternating 31 ↔ 64 reproduces it verbatim. */
const val NARROW_COLS = 31
const val WIDE_COLS = 64
const val ROWS = 24
/** Deep enough that a reflow takes real time (the race window), shallow enough to stay fast. */
const val TEST_TRANSCRIPT_ROWS = 4_000
/** The confined-thread version threw within a couple of iterations; 40 leaves no room for luck. */
const val REGRID_ITERATIONS = 40
const val AWAIT_TIMEOUT_SECONDS = 30L
const val POLL_INTERVAL_MS = 5L
/** Plain glyphs plus a newline, so the reflow has content to wrap rather than an empty grid. */
val SAMPLE_OUTPUT: ByteArray = ("web terminal regrid probe ".repeat(8) + "\r\n").toByteArray()
/** `OSC 0 ; title BEL` — parsed inside `append`, which is what makes the title a thread probe. */
val TITLE_SEQUENCE: ByteArray = "\u001b]0;probe\u0007".toByteArray()
}
}