Skip to content

Commit 93392fc

Browse files
authored
Fix terminal state and key state data races (#335)
* Update gitignore to ignore .swp * Fix terminal state data races 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) * Fix KeyPressed data race 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
1 parent 62453da commit 93392fc

8 files changed

Lines changed: 48 additions & 20 deletions

File tree

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
.idea
22
.vscode
33
/darktile
4+
*.swp

internal/app/darktile/gui/gui.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,12 +102,12 @@ func (g *GUI) Run() error {
102102
func (g *GUI) watchForUpdate() {
103103
for range g.updateChan {
104104
ebiten.ScheduleFrame()
105-
go func() {
106-
if g.keyState.AnythingPressed() {
105+
if g.keyState.AnythingPressed() {
106+
go func() {
107107
time.Sleep(time.Millisecond * 10)
108108
ebiten.ScheduleFrame()
109-
}
110-
}()
109+
}()
110+
}
111111
}
112112
}
113113

internal/app/darktile/gui/input.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ var modifiableKeys = map[ebiten.Key]uint8{
3737
}
3838

3939
func (g *GUI) handleInput() error {
40+
g.terminal.Lock()
41+
defer g.terminal.Unlock()
4042

4143
if err := g.handleMouse(); err != nil {
4244
return err

internal/app/darktile/gui/key_states.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ type press struct {
3030
}
3131

3232
func (k *keyState) AnythingPressed() bool {
33+
k.mu.Lock()
34+
defer k.mu.Unlock()
3335
return len(k.keys) > 0
3436
}
3537

internal/app/darktile/gui/render/render.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,8 @@ func New(screen *ebiten.Image, terminal *termutil.Terminal, fontManager *font.Ma
6262
}
6363

6464
func (r *Render) Draw() {
65+
r.terminal.Lock()
66+
defer r.terminal.Unlock()
6567

6668
// 1. fill frame with default background colour
6769
r.frame.Fill(r.theme.DefaultBackground())

internal/app/darktile/gui/resize.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,17 @@ func (g *GUI) Layout(outsideWidth, outsideHeight int) (int, int) {
2222

2323
func (g *GUI) resize(w, h int) {
2424

25-
if g.fontManager.CharSize().X == 0 || g.fontManager.CharSize().Y == 0 {
25+
if g.fontManager.CharSize().X == 0 || g.fontManager.CharSize().Y == 0 || g.terminal == nil {
2626
return
2727
}
2828

2929
cols := uint16(w / g.fontManager.CharSize().X)
3030
rows := uint16(h / g.fontManager.CharSize().Y)
3131

32-
if g.terminal != nil && g.terminal.IsRunning() {
32+
g.terminal.Lock()
33+
defer g.terminal.Unlock()
34+
35+
if g.terminal.IsRunning() {
3336
_ = g.terminal.SetSize(rows, cols)
3437
}
3538
}

internal/app/darktile/termutil/ansi.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ func (t *Terminal) handleANSI(readChan chan MeasuredRune) (renderRequired bool)
66

77
t.log("ANSI SEQ %c 0x%X", r.Rune, r.Rune)
88

9+
t.mu.Lock()
10+
defer t.mu.Unlock()
11+
912
switch r.Rune {
1013
case '[':
1114
return t.handleCSI(readChan)

internal/app/darktile/termutil/terminal.go

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"io"
88
"os"
99
"os/exec"
10+
"sync"
1011

1112
"github.com/creack/pty"
1213
"golang.org/x/term"
@@ -20,6 +21,7 @@ const (
2021

2122
// Terminal communicates with the underlying terminal
2223
type Terminal struct {
24+
mu sync.Mutex
2325
windowManipulator WindowManipulator
2426
pty *os.File
2527
updateChan chan struct{}
@@ -196,24 +198,29 @@ func (t *Terminal) requestRender() {
196198
}
197199
}
198200

201+
func (t *Terminal) processSequence(mr MeasuredRune) (render bool) {
202+
if mr.Rune == 0x1b {
203+
return t.handleANSI(t.processChan)
204+
}
205+
return t.processRunes(mr)
206+
}
207+
199208
func (t *Terminal) process() {
200209
for {
201210
select {
202211
case <-t.closeChan:
203212
return
204213
case mr := <-t.processChan:
205-
if mr.Rune == 0x1b { // ANSI escape char, which means this is a sequence
206-
if t.handleANSI(t.processChan) {
207-
t.requestRender()
208-
}
209-
} else if t.processRunes(mr) { // otherwise it's just an individual rune we need to process
214+
if t.processSequence(mr) {
210215
t.requestRender()
211216
}
212217
}
213218
}
214219
}
215220

216221
func (t *Terminal) processRunes(runes ...MeasuredRune) (renderRequired bool) {
222+
t.mu.Lock()
223+
defer t.mu.Unlock()
217224

218225
for _, r := range runes {
219226

@@ -226,31 +233,31 @@ func (t *Terminal) processRunes(runes ...MeasuredRune) (renderRequired bool) {
226233
//DING DING DING
227234
continue
228235
case 0x8: //backspace
229-
t.GetActiveBuffer().backspace()
236+
t.activeBuffer.backspace()
230237
renderRequired = true
231238
case 0x9: //tab
232-
t.GetActiveBuffer().tab()
239+
t.activeBuffer.tab()
233240
renderRequired = true
234241
case 0xa, 0xc: //newLine/form feed
235-
t.GetActiveBuffer().newLine()
242+
t.activeBuffer.newLine()
236243
renderRequired = true
237244
case 0xb: //vertical tab
238-
t.GetActiveBuffer().verticalTab()
245+
t.activeBuffer.verticalTab()
239246
renderRequired = true
240247
case 0xd: //carriageReturn
241-
t.GetActiveBuffer().carriageReturn()
248+
t.activeBuffer.carriageReturn()
242249
renderRequired = true
243250
case 0xe: //shiftOut
244-
t.GetActiveBuffer().currentCharset = 1
251+
t.activeBuffer.currentCharset = 1
245252
case 0xf: //shiftIn
246-
t.GetActiveBuffer().currentCharset = 0
253+
t.activeBuffer.currentCharset = 0
247254
default:
248255
if r.Rune < 0x20 {
249256
// handle any other control chars here?
250257
continue
251258
}
252259

253-
t.GetActiveBuffer().write(t.translateRune(r))
260+
t.activeBuffer.write(t.translateRune(r))
254261
renderRequired = true
255262
}
256263
}
@@ -259,7 +266,7 @@ func (t *Terminal) processRunes(runes ...MeasuredRune) (renderRequired bool) {
259266
}
260267

261268
func (t *Terminal) translateRune(b MeasuredRune) MeasuredRune {
262-
table := t.GetActiveBuffer().charsets[t.GetActiveBuffer().currentCharset]
269+
table := t.activeBuffer.charsets[t.activeBuffer.currentCharset]
263270
if table == nil {
264271
return b
265272
}
@@ -306,3 +313,11 @@ func (t *Terminal) useMainBuffer() {
306313
func (t *Terminal) useAltBuffer() {
307314
t.switchBuffer(AltBuffer)
308315
}
316+
317+
func (t *Terminal) Lock() {
318+
t.mu.Lock()
319+
}
320+
321+
func (t *Terminal) Unlock() {
322+
t.mu.Unlock()
323+
}

0 commit comments

Comments
 (0)