Fix terminal state and key state data races #335
Merged
Conversation
This file contains 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
go run -race cmd/darktile/main.go WARNING: DATA RACE Write at 0x00c000864128 by goroutine 23: github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).sgrSequenceHandler() /home/ernestrc/src/darktile/internal/app/darktile/termutil/csi.go:973 +0x12ca github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).handleCSI() /home/ernestrc/src/darktile/internal/app/darktile/termutil/csi.go:62 +0x7e8 github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).handleANSI() /home/ernestrc/src/darktile/internal/app/darktile/termutil/ansi.go:11 +0xc86 github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).process() /home/ernestrc/src/darktile/internal/app/darktile/termutil/terminal.go:206 +0x126 github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).Run·dwrap·10() /home/ernestrc/src/darktile/internal/app/darktile/termutil/terminal.go:171 +0x39 Previous read at 0x00c000864128 by goroutine 22: [failed to restore the stack] Goroutine 23 (running) created at: github.com/liamg/darktile/internal/app/darktile/termutil.(*Terminal).Run() /home/ernestrc/src/darktile/internal/app/darktile/termutil/terminal.go:171 +0x517 github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).Run.func1() /home/ernestrc/src/darktile/internal/app/darktile/gui/gui.go:80 +0xa Goroutine 22 (running) created at: github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw.(*UserInterface).Run() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw/run_notsinglethread.go:37 +0x2c4 github.com/hajimehoshi/ebiten/v2.RunGame() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/run.go:158 +0x1d4 github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).Run() /home/ernestrc/src/darktile/internal/app/darktile/gui/gui.go:99 +0x2de github.com/liamg/darktile/internal/app/darktile/cmd.glob..func2() /home/ernestrc/src/darktile/internal/app/darktile/cmd/root.go:130 +0x13a9 github.com/spf13/cobra.(*Command).execute() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:852 +0xa7d github.com/spf13/cobra.(*Command).ExecuteC() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:960 +0x5da github.com/spf13/cobra.(*Command).Execute() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:897 +0x366 github.com/liamg/darktile/internal/app/darktile/cmd.Execute() /home/ernestrc/src/darktile/internal/app/darktile/cmd/root.go:153 +0x34f main.main() /home/ernestrc/src/darktile/cmd/darktile/main.go:75 +0x24 ================== Found 26 data race(s)
go run -race cmd/darktile/main.go WARNING: DATA RACE Write at 0x00c00009fbc0 by goroutine 20: runtime.mapdelete_fast64() /usr/lib/go/src/runtime/map_fast64.go:272 +0x0 github.com/liamg/darktile/internal/app/darktile/gui.(*keyState).RepeatPressed() /home/ernestrc/src/darktile/internal/app/darktile/gui/key_states.go:61 +0x428 github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).handleInput() /home/ernestrc/src/darktile/internal/app/darktile/gui/input.go:109 +0x224 github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).Update() /home/ernestrc/src/darktile/internal/app/darktile/gui/update.go:34 +0x2e github.com/hajimehoshi/ebiten/v2.(*imageDumper).update() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/imagedumper_desktop.go:111 +0x85 github.com/hajimehoshi/ebiten/v2.(*imageDumperGame).Update() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/run.go:115 +0x172 github.com/hajimehoshi/ebiten/v2.(*uiContext).updateImpl() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/uicontext.go:194 +0x12c github.com/hajimehoshi/ebiten/v2.(*uiContext).update() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/uicontext.go:166 +0x88 github.com/hajimehoshi/ebiten/v2.(*uiContext).Update() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/uicontext.go:147 +0x3b github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw.(*UserInterface).loop() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw/ui.go:1036 +0x401 github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw.(*UserInterface).Run.func1() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw/run_notsinglethread.go:53 +0x1b1 Previous read at 0x00c00009fbc0 by goroutine 19: github.com/liamg/darktile/internal/app/darktile/gui.(*keyState).AnythingPressed() /home/ernestrc/src/darktile/internal/app/darktile/gui/key_states.go:33 +0xbc github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).watchForUpdate() /home/ernestrc/src/darktile/internal/app/darktile/gui/gui.go:105 +0x77 github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).Run·dwrap·2() /home/ernestrc/src/darktile/internal/app/darktile/gui/gui.go:97 +0x39 Goroutine 20 (running) created at: github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw.(*UserInterface).Run() /home/ernestrc/src/darktile/vendor/github.com/hajimehoshi/ebiten/v2/internal/uidriver/glfw/run_notsinglethread.go:37 +0x2c4 github.com/hajimehoshi/ebiten/v2.RunGame() Goroutine 19 (running) created at: github.com/liamg/darktile/internal/app/darktile/gui.(*GUI).Run() /home/ernestrc/src/darktile/internal/app/darktile/gui/gui.go:97 +0x2ca github.com/liamg/darktile/internal/app/darktile/cmd.glob..func2() /home/ernestrc/src/darktile/internal/app/darktile/cmd/root.go:130 +0x13a9 github.com/spf13/cobra.(*Command).execute() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:852 +0xa7d github.com/spf13/cobra.(*Command).ExecuteC() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:960 +0x5da github.com/spf13/cobra.(*Command).Execute() /home/ernestrc/src/darktile/vendor/github.com/spf13/cobra/command.go:897 +0x366 github.com/liamg/darktile/internal/app/darktile/cmd.Execute() /home/ernestrc/src/darktile/internal/app/darktile/cmd/root.go:153 +0x34f main.main() /home/ernestrc/src/darktile/cmd/darktile/main.go:75 +0x24
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I am using darktile for an internal project and stumbled upon a few data races. In essence, we are processing input via
io.Copy->Terminal.Write->handleInputconcurrently without synchronizing with the main rendering loop (GUI.Update->...->g.terminal.GetActiveBuffer).I haven't done any analysis on contention but at first glance we might be adding some, since we are blocking on internal calls to
WriteToPtyetc. This solution was enough for my purposes so feel free to close PR if you think a more sophisticated approach is required.