Filter PTY malloc noise and add flush observability#184
Conversation
| processedBytes := len(chunk) | ||
| filteredLen := 0 | ||
| filterApplied := false | ||
| tab.mu.Lock() | ||
| if tab.Terminal != nil { | ||
| flushDone := perf.Time("pty_flush") | ||
| tab.Terminal.Write(chunk) | ||
| flushDone() | ||
| perf.Count("pty_flush_bytes", int64(len(chunk))) | ||
| filtered := common.FilterKnownPTYNoiseStream(chunk, &tab.ptyNoiseTrailing) | ||
| filteredLen = len(filtered) | ||
| filterApplied = true | ||
| if len(filtered) > 0 { | ||
| flushDone := perf.Time("pty_flush") | ||
| tab.Terminal.Write(filtered) | ||
| flushDone() | ||
| perf.Count("pty_flush_bytes", int64(len(filtered))) | ||
| } | ||
| // Activity state intentionally tracks visible terminal mutations only. | ||
| // Noise-only chunks are filtered above and must not update activity tags. | ||
| // We still run this to clear pending visible state when no mutation occurred. | ||
| tagSessionName, tagTimestamp, _ = m.noteVisibleActivityLocked(tab, hasMoreBuffered, visibleSeq) | ||
| } | ||
| tab.mu.Unlock() | ||
| perf.Count("pty_flush_bytes_processed", int64(processedBytes)) | ||
| if filterApplied { | ||
| filteredBytes := processedBytes - filteredLen | ||
| if filteredBytes > 0 { | ||
| perf.Count("pty_flush_bytes_filtered", int64(filteredBytes)) | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 Out-of-order ptyNoiseTrailing state corruption when tab actor channel is full
The new ptyNoiseTrailing filter state is shared between two code paths that can process chunks out of order: the tab actor goroutine and the main Bubble Tea goroutine's fallback path. When sendTabEvent succeeds for chunk A but fails for a subsequent chunk B (channel full), the main goroutine processes chunk B directly (under tab.mu) while chunk A remains queued in the tab actor channel. Chunk B updates ptyNoiseTrailing before chunk A reads it, causing out-of-order state updates.
Detailed Explanation of the Race
The tab actor processes events from a channel (m.tabEvents) on a separate goroutine. When the channel is full, sendTabEvent returns false and the caller falls back to processing the chunk synchronously on the main goroutine.
Consider this sequence:
updatePTYFlushextracts chunk1, callssendTabEvent→ succeeds (chunk1 enqueued)updatePTYFlushextracts chunk2, callssendTabEvent→ fails (channel full)- Main goroutine acquires
tab.mu, processes chunk2 viaFilterKnownPTYNoiseStream(chunk2, &tab.ptyNoiseTrailing), updatesptyNoiseTrailingto state S2 - Tab actor dequeues chunk1, acquires
tab.mu, processes chunk1 viaFilterKnownPTYNoiseStream(chunk1, &tab.ptyNoiseTrailing)— but nowptyNoiseTrailingcontains state S2 from chunk2
At step 4, the tab actor prepends chunk2's trailing fragment to chunk1's data. This corrupts the terminal output: bytes from a later chunk appear at the start of an earlier chunk's rendering.
The same pattern exists in the else branch at model_input_lifecycle_pty.go:262-290 (when tab actor is not ready) — though that path is less likely to interleave with the tab actor.
Impact: Under back-pressure (tab actor channel full), terminal output can contain misplaced bytes from the noise-filter's trailing buffer. In practice this is rare since ptyNoiseTrailing is usually empty, but when it is non-empty (a potential diagnostic fragment is being buffered), the corruption is real.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Describe the change and intended behavior.
Quality Checklist
make devchecklocally.make lint-strict-newlocally for changed code.make harness-presets.go test ./internal/tmux ./internal/e2e.