fix(app): guard the dirty-boundary set against concurrent marks - #203
Merged
Conversation
Marking a boundary dirty is locked at every hop but this one: SetNeedsRedraw, InvalidateScene and RegisterDirtyBoundary each take their mutex, snapshot, and release before calling out. AddDirtyBoundary then writes a bare map. That is the call pattern of any app whose content arrives on its own goroutines — a terminal pane per shell, a video surface, a download. With one such goroutine the race stayed invisible; with two the runtime takes the process down with "concurrent map writes", which no recover can catch. An uncontended mutex per mark, against a map write that was already there.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
kolkov
approved these changes
Aug 3, 2026
kolkov
left a comment
kolkov
left a comment
Contributor
There was a problem hiding this comment.
Validated: Flutter is single-threaded (no locks), Qt serializes through event queue. sync.Mutex is the correct Go equivalent — RWMutex overhead not justified for nanosecond critical sections. No other unprotected maps in Window. Closes a real production crash (concurrent map writes from multi-pane terminal).
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
2 participants
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.
Marking a boundary dirty is locked at every hop but the last one.
SetNeedsRedraw,InvalidateSceneandRegisterDirtyBoundaryeach take their mutex, snapshot, and release it before calling out — the shape of code meant to be reached from another goroutine.Window.AddDirtyBoundarythen writes a bare map.That is the call pattern of any app whose content arrives on its own goroutines: a terminal pane per shell, a video surface, a download. With one such goroutine the race stays invisible. With two, the runtime takes the process down with
concurrent map writes, which norecovercan catch.Found in a terminal emulator on
gogpu/uiwhere each pane repaints from its own PTY reader: one pane was fine for weeks, two panes printing at once was a hard crash.The fix is a mutex on the four methods that touch
dirtyBoundaries— an uncontended lock per mark, against a map write that was already there.Rebased onto v0.1.49.
go build ./...andgo test ./app/... ./widget/...pass.