diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index f59944e..8a40976 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -1 +1 @@ -* @liamg +* @liamg @MaxRis diff --git a/gui/gui.go b/gui/gui.go index bbeecdf..28cb66a 100644 --- a/gui/gui.go +++ b/gui/gui.go @@ -173,15 +173,23 @@ func (gui *GUI) scale() float32 { } // can only be called on OS thread -func (gui *GUI) resizeToTerminal(newCols uint, newRows uint) { +func (gui *GUI) resizeToTerminal() { if gui.window.GetAttrib(glfw.Iconified) != 0 { return } + // Order of locking: + // 1. resizeLock + // 2. terminal's lock gui.resizeLock.Lock() defer gui.resizeLock.Unlock() + gui.terminal.Lock() + defer gui.terminal.Unlock() + termCols, termRows := gui.terminal.GetSize() + newCols := uint(termCols) + newRows := uint(termRows) cols, rows := gui.renderer.GetTermSize() if cols == newCols && rows == newRows { return @@ -244,9 +252,16 @@ func (gui *GUI) resize(w *glfw.Window, width int, height int) { return } + // Order of locking: + // 1. resizeLock + // 2. terminal's lock + terminalAlreadyLocked := false if gui.internalResize == false { gui.resizeLock.Lock() defer gui.resizeLock.Unlock() + // No need to lock the terminal right away, we can lock it later + } else { + terminalAlreadyLocked = true } gui.logger.Debugf("Initiating GUI resize to %dx%d", width, height) @@ -268,6 +283,11 @@ func (gui *GUI) resize(w *glfw.Window, width int, height int) { gui.logger.Debugf("Calculating size in cols/rows...") cols, rows := gui.renderer.GetTermSize() gui.logger.Debugf("Resizing internal terminal...") + if !terminalAlreadyLocked { + gui.terminal.Lock() + defer gui.terminal.Unlock() + terminalAlreadyLocked = true + } if err := gui.terminal.SetSize(cols, rows); err != nil { gui.logger.Errorf("Failed to resize terminal to %d cols, %d rows: %s", cols, rows, err) } @@ -278,11 +298,16 @@ func (gui *GUI) resize(w *glfw.Window, width int, height int) { gui.logger.Debugf("Setting viewport size...") gl.Viewport(0, 0, int32(gui.width), int32(gui.height)) + if !terminalAlreadyLocked { + gui.terminal.Lock() + defer gui.terminal.Unlock() + terminalAlreadyLocked = true + } gui.terminal.SetCharSize(gui.renderer.cellWidth, gui.renderer.cellHeight) gui.logger.Debugf("Resize complete!") - gui.redraw() + gui.redraw(!terminalAlreadyLocked) gui.window.SwapBuffers() } @@ -337,11 +362,11 @@ func (gui *GUI) Render() error { gui.window.SetMouseButtonCallback(gui.mouseButtonCallback) gui.window.SetCursorPosCallback(gui.mouseMoveCallback) gui.window.SetRefreshCallback(func(w *glfw.Window) { - gui.terminal.SetDirty() + gui.terminal.SetDirtyLocked() }) gui.window.SetFocusCallback(func(w *glfw.Window, focused bool) { if focused { - gui.terminal.SetDirty() + gui.terminal.SetDirtyLocked() } }) gui.window.SetPosCallback(gui.windowPosChangeCallback) @@ -354,6 +379,10 @@ func (gui *GUI) Render() error { gui.resize(gui.window, w, h) } + gui.terminal.AttachTitleChangeHandler(titleChan) + gui.terminal.AttachResizeHandler(resizeChan) + gui.terminal.AttachReverseHandler(reverseChan) + gui.logger.Debugf("Starting pty read handling...") go func() { @@ -372,10 +401,6 @@ func (gui *GUI) Render() error { gl.Disable(gl.DEPTH_TEST) gl.TexParameterf(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST) - gui.terminal.AttachTitleChangeHandler(titleChan) - gui.terminal.AttachResizeHandler(resizeChan) - gui.terminal.AttachReverseHandler(reverseChan) - ticker := time.NewTicker(time.Second) defer ticker.Stop() @@ -409,8 +434,7 @@ func (gui *GUI) Render() error { case <-titleChan: gui.window.SetTitle(gui.terminal.GetTitle()) case <-resizeChan: - cols, rows := gui.terminal.GetSize() - gui.resizeToTerminal(uint(cols), uint(rows)) + gui.resizeToTerminal() case reverse := <-reverseChan: gui.generateDefaultCell(reverse) forceRedraw = true @@ -421,7 +445,7 @@ func (gui *GUI) Render() error { if gui.terminal.CheckDirty() || forceRedraw { - gui.redraw() + gui.redraw(true) if gui.showDebugInfo { gui.textbox(2, 2, fmt.Sprintf(`Cursor: %d,%d @@ -441,7 +465,7 @@ Buffer Size: %d lines if showMessage { if latestVersion != "" && time.Since(startTime) < time.Second*10 && gui.terminal.ActiveBuffer().RawLine() == 0 { - time.AfterFunc(time.Second, gui.terminal.SetDirty) + time.AfterFunc(time.Second, gui.terminal.SetDirtyLocked) _, h := gui.terminal.GetSize() var msg string if version.Version == "" { @@ -471,8 +495,11 @@ Buffer Size: %d lines } -func (gui *GUI) redraw() { - gl.Clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT | gl.STENCIL_BUFFER_BIT) +func (gui *GUI) renderTerminalData(shouldLock bool) { + if shouldLock { + gui.terminal.Lock() + defer gui.terminal.Unlock() + } lines := gui.terminal.GetVisibleLines() lineCount := int(gui.terminal.ActiveBuffer().ViewHeight()) colCount := int(gui.terminal.ActiveBuffer().ViewWidth()) @@ -602,6 +629,11 @@ func (gui *GUI) redraw() { } } +} + +func (gui *GUI) redraw(shouldLock bool) { + gl.Clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT | gl.STENCIL_BUFFER_BIT) + gui.renderTerminalData(shouldLock) gui.renderOverlay() } diff --git a/gui/overlays.go b/gui/overlays.go index e1d32dc..b792fa3 100644 --- a/gui/overlays.go +++ b/gui/overlays.go @@ -5,7 +5,7 @@ type overlay interface { } func (gui *GUI) setOverlay(m overlay) { - defer gui.terminal.SetDirty() + defer gui.terminal.SetDirtyLocked() gui.overlay = m } diff --git a/main_test.go b/main_test.go index 3a8479e..5ef9b88 100644 --- a/main_test.go +++ b/main_test.go @@ -105,7 +105,11 @@ func TestCursorMovement(t *testing.T) { send(term, "1\n") sleep() - if term.ActiveBuffer().CompareViewLines("vttest/test-cursor-movement-1") == false { + term.Lock() + compareResult := term.ActiveBuffer().CompareViewLines("vttest/test-cursor-movement-1") + term.Unlock() + + if compareResult == false { os.Exit(terminate(fmt.Sprintf("ActiveBuffer doesn't match vttest template vttest/test-cursor-movement-1"))) } diff --git a/terminal/ansi.go b/terminal/ansi.go index 3dae73d..e56f697 100644 --- a/terminal/ansi.go +++ b/terminal/ansi.go @@ -35,26 +35,42 @@ func swallowHandler(n int) func(pty chan rune, terminal *Terminal) error { } func risHandler(pty chan rune, terminal *Terminal) error { + terminal.Lock() + defer terminal.Unlock() + terminal.ActiveBuffer().Clear() return nil } func indexHandler(pty chan rune, terminal *Terminal) error { + terminal.Lock() + defer terminal.Unlock() + terminal.ActiveBuffer().Index() return nil } func reverseIndexHandler(pty chan rune, terminal *Terminal) error { + terminal.Lock() + defer terminal.Unlock() + terminal.ActiveBuffer().ReverseIndex() return nil } func saveCursorHandler(pty chan rune, terminal *Terminal) error { + // Handler should lock the terminal if there will be write operations to any data read by the renderer + // terminal.Lock() + // defer terminal.Unlock() + terminal.ActiveBuffer().SaveCursor() return nil } func restoreCursorHandler(pty chan rune, terminal *Terminal) error { + terminal.Lock() + defer terminal.Unlock() + terminal.ActiveBuffer().RestoreCursor() return nil } @@ -73,11 +89,18 @@ func ansiHandler(pty chan rune, terminal *Terminal) error { } func nextLineHandler(pty chan rune, terminal *Terminal) error { + terminal.Lock() + defer terminal.Unlock() + terminal.ActiveBuffer().NewLineEx(true) return nil } func tabSetHandler(pty chan rune, terminal *Terminal) error { + // Handler should lock the terminal if there will be write operations to any data read by the renderer + // terminal.Lock() + // defer terminal.Unlock() + terminal.terminalState.TabSetAtCursor() return nil } diff --git a/terminal/charset.go b/terminal/charset.go index 08c2f2a..e53e200 100644 --- a/terminal/charset.go +++ b/terminal/charset.go @@ -54,6 +54,10 @@ func scs1Handler(pty chan rune, terminal *Terminal) error { func scsHandler(pty chan rune, terminal *Terminal, which int) error { b := <-pty + // Handler should lock the terminal if there will be write operations to any data read by the renderer + // terminal.Lock() + // defer terminal.Unlock() + cs, ok := charSets[b] if ok { terminal.logger.Debugf("Selected charset %v into G%v", string(b), which) diff --git a/terminal/csi.go b/terminal/csi.go index 303dddd..6a0e3f8 100644 --- a/terminal/csi.go +++ b/terminal/csi.go @@ -89,6 +89,9 @@ func splitParams(paramString string) []string { func csiHandler(pty chan rune, terminal *Terminal) error { final, param, intermediate := loadCSI(pty) + terminal.Lock() + defer terminal.Unlock() + // process intermediate control codes before the CSI for _, b := range intermediate { terminal.processRune(b) diff --git a/terminal/output.go b/terminal/output.go index a528263..3425c9f 100644 --- a/terminal/output.go +++ b/terminal/output.go @@ -72,6 +72,13 @@ func shiftInHandler(terminal *Terminal) error { return nil } +func (terminal *Terminal) processRuneLocked(b rune) { + terminal.Lock() + defer terminal.Unlock() + + terminal.processRune(b) +} + func (terminal *Terminal) processRune(b rune) { if handler, ok := runeMap[b]; ok { if err := handler(terminal); err != nil { @@ -103,6 +110,8 @@ func (terminal *Terminal) processInput(pty chan rune) { var b rune + // debug := "" + for { if terminal.config.Slomo { @@ -111,15 +120,19 @@ func (terminal *Terminal) processInput(pty chan rune) { b = <-pty + // debug += fmt.Sprintf("0x%x ", b) + if b == 0x1b { + // terminal.logger.Debug(debug) + // debug = "" //terminal.logger.Debugf("Handling escape sequence: 0x%x", b) if err := ansiHandler(pty, terminal); err != nil { terminal.logger.Errorf("Error handling escape sequence: %s", err) } - terminal.isDirty = true + terminal.SetDirtyLocked() continue } - terminal.processRune(b) + terminal.processRuneLocked(b) } } diff --git a/terminal/scr_state.go b/terminal/scr_state.go index ae1a346..fabe6c7 100644 --- a/terminal/scr_state.go +++ b/terminal/scr_state.go @@ -6,6 +6,9 @@ func screenStateHandler(pty chan rune, terminal *Terminal) error { b := <-pty switch b { case '8': // DECALN -- Screen Alignment Pattern + terminal.Lock() + defer terminal.Unlock() + // hide cursor? buffer := terminal.ActiveBuffer() terminal.ResetVerticalMargins() diff --git a/terminal/sixel.go b/terminal/sixel.go index 4c452bc..800d26e 100644 --- a/terminal/sixel.go +++ b/terminal/sixel.go @@ -131,6 +131,9 @@ func sixelHandler(pty chan rune, terminal *Terminal) error { defer terminal.SetLineFeedMode() } + terminal.Lock() + defer terminal.Unlock() + drawSixel(six, terminal) return nil diff --git a/terminal/terminal.go b/terminal/terminal.go index 1e117f0..6449ff5 100644 --- a/terminal/terminal.go +++ b/terminal/terminal.go @@ -105,6 +105,9 @@ func (terminal *Terminal) SetBracketedPasteMode(enabled bool) { } func (terminal *Terminal) CheckDirty() bool { + terminal.Lock() + defer terminal.Unlock() + d := terminal.isDirty terminal.isDirty = false return d || terminal.ActiveBuffer().IsDirty() @@ -355,9 +358,6 @@ func (terminal *Terminal) GetSize() (int, int) { } func (terminal *Terminal) SetSize(newCols uint, newLines uint) error { - terminal.lock.Lock() - defer terminal.lock.Unlock() - if terminal.size.Width == uint16(newCols) && terminal.size.Height == uint16(newLines) { return nil } @@ -420,3 +420,20 @@ func (terminal *Terminal) SetScreenMode(enabled bool) { } terminal.emitReverse(enabled) } + +func (terminal *Terminal) Lock() { + terminal.lock.Lock() +} + +func (terminal *Terminal) Unlock() { + terminal.lock.Unlock() +} + +// SetDirtyLocked sets dirty flag locking the terminal to prevent data race warnings +// @todo remove when switching to event-driven architecture +func (terminal *Terminal) SetDirtyLocked() { + terminal.Lock() + defer terminal.Unlock() + + terminal.SetDirty() +}