Fix terminal resize race conditions

The runebuf, search and complete data structures all maintain their
own cache of the terminal width and height. When the terminal is
resized, a signal is sent to a go routine which calls a function
that sets the new width and height to each data structure instance.
However, this races with the main thread reading the sizes.

Instead of introducing more locks, it makes sense that the terminal
itself caches it's width and height and the other structures just
get it as necessary. This removes all the racing.

As part of this change, search, complete and runebuf constructor
changes to no longer require the initial sizes. Also each structure
needs a reference to the Terminal so they can get the width/height.
As the io.Writer parameter is actually the terminal anyway, the
simpliest option was just to change the type from the io.Writer to
Terminal. I don't believe that anyone would be calling these
functions directly so the signature changes should be ok. I also
removed the no longer used OnWidthChange() and OnSizeChange()
functions from these three structures.
This commit is contained in:
Thomas O'Dowd 2023-02-28 11:34:47 +09:00
parent 86f47fbf9e
commit c30c5a000a
5 changed files with 77 additions and 97 deletions

View File

@ -4,7 +4,6 @@ import (
"bufio"
"bytes"
"fmt"
"io"
)
type AutoCompleter interface {
@ -25,10 +24,8 @@ func (t *TabCompleter) Do([]rune, int) ([][]rune, int) {
}
type opCompleter struct {
w io.Writer
w *Terminal
op *Operation
width int
height int
inCompleteMode bool
inSelectMode bool
@ -42,12 +39,10 @@ type opCompleter struct {
candidateColWidth int // width of candidate columns
}
func newOpCompleter(w io.Writer, op *Operation, width, height int) *opCompleter {
func newOpCompleter(w *Terminal, op *Operation) *opCompleter {
return &opCompleter{
w: w,
op: op,
width: width,
height: height,
}
}
@ -73,7 +68,8 @@ func (o *opCompleter) nextCandidate(i int) {
// when tab pressed if cannot do complete for reason such as width unknown
// or no candidates available.
func (o *opCompleter) OnComplete() (ringBell bool) {
if o.width == 0 || o.height < 3 {
tWidth, tHeight := o.w.GetWidthHeight()
if tWidth == 0 || tHeight < 3 {
return false
}
if o.IsInCompleteSelectMode() {
@ -103,7 +99,7 @@ func (o *opCompleter) OnComplete() (ringBell bool) {
if len(newLines) == 0 || (len(newLines) == 1 && len(newLines[0]) == 0) {
o.ExitCompleteMode(false)
return false // will ring bell on initial tab press
}
}
if o.candidateOff > offset {
// part of buffer we are completing has changed. Example might be that we were completing "ls" and
// user typed space so we are no longer completing "ls" but now we are completing an argument of
@ -246,15 +242,6 @@ func (o *opCompleter) getMatrixSize() int {
return line * colNum
}
func (o *opCompleter) OnWidthChange(newWidth int) {
o.width = newWidth
}
func (o *opCompleter) OnSizeChange(newWidth, newHeight int) {
o.width = newWidth
o.height = newHeight
}
// setColumnInfo calculates column width and number of columns required
// to present the list of candidates on the terminal.
func (o *opCompleter) setColumnInfo() {
@ -270,8 +257,10 @@ func (o *opCompleter) setColumnInfo() {
}
colWidth++ // whitespace between cols
tWidth, _ := o.w.GetWidthHeight()
// -1 to avoid end of line issues
width := o.width - 1
width := tWidth - 1
colNum := width / colWidth
if colNum != 0 {
colWidth += (width - (colWidth * colNum)) / colNum
@ -283,8 +272,9 @@ func (o *opCompleter) setColumnInfo() {
// needPagerMode returns true if number of candidates would go off the page
func (o *opCompleter) needPagerMode() bool {
tWidth, tHeight := o.w.GetWidthHeight()
buflineCnt := o.op.buf.LineCount() // lines taken by buffer content
linesAvail := o.height - buflineCnt // lines available without scrolling buffer off screen
linesAvail := tHeight - buflineCnt // lines available without scrolling buffer off screen
if o.candidateColNum > 0 {
// Normal case where each candidate at least fits on a line
maxOrPage := linesAvail * o.candidateColNum // max candiates without needing to page
@ -299,9 +289,9 @@ func (o *opCompleter) needPagerMode() bool {
for _, c := range o.candidate {
cWidth := sameWidth + runes.WidthAll(c)
cLines := 1
if o.width > 0 {
cLines = cWidth / o.width
if cWidth % o.width > 0 {
if tWidth > 0 {
cLines = cWidth / tWidth
if cWidth % tWidth > 0 {
cLines++
}
}
@ -326,6 +316,7 @@ func (o *opCompleter) CompleteRefresh() {
buf.WriteString("\033[J")
same := o.op.buf.RuneSlice(-o.candidateOff)
tWidth, _ := o.w.GetWidthHeight()
colIdx := 0
lines := 0
@ -334,13 +325,13 @@ func (o *opCompleter) CompleteRefresh() {
inSelect := idx == o.candidateChoise && o.IsInCompleteSelectMode()
cWidth := sameWidth + runes.WidthAll(c)
cLines := 1
if o.width > 0 {
if tWidth > 0 {
sWidth := 0
if isWindows && inSelect {
sWidth = 1 // adjust for hightlighting on Windows
}
cLines = (cWidth + sWidth) / o.width
if (cWidth + sWidth) % o.width > 0 {
cLines = (cWidth + sWidth) / tWidth
if (cWidth + sWidth) % tWidth > 0 {
cLines++
}
}
@ -403,11 +394,12 @@ func (o *opCompleter) pagerRefresh() (stayInMode bool) {
} else {
// after first page, redraw over --More--
buf.WriteString("\r")
}
}
buf.WriteString("\033[J") // clear anything below
same := o.op.buf.RuneSlice(-o.candidateOff)
sameWidth := runes.WidthAll(same)
tWidth, tHeight := o.w.GetWidthHeight()
colIdx := 0
lines := 1
@ -415,13 +407,13 @@ func (o *opCompleter) pagerRefresh() (stayInMode bool) {
c := o.candidate[o.candidateChoise]
cWidth := sameWidth + runes.WidthAll(c)
cLines := 1
if o.width > 0 {
cLines = cWidth / o.width
if cWidth % o.width > 0 {
if tWidth > 0 {
cLines = cWidth / tWidth
if cWidth % tWidth > 0 {
cLines++
}
}
if lines > 1 && lines + cLines > o.height {
if lines > 1 && lines + cLines > tHeight {
break // won't fit on page, stop early.
}
buf.WriteString(string(same))
@ -460,7 +452,8 @@ func (o *opCompleter) pagerRefresh() (stayInMode bool) {
// we rewrite the prompt it does not over write the page content. The code to rewrite
// the prompt assumes the cursor is at the index line, so we add enough blank lines.
func (o *opCompleter) scrollOutOfPagerMode() {
lineCnt := o.op.buf.IdxLine(o.width)
tWidth, _ := o.w.GetWidthHeight()
lineCnt := o.op.buf.IdxLine(tWidth)
if lineCnt > 0 {
buf := bufio.NewWriter(o.w)
buf.Write(bytes.Repeat([]byte("\n"), lineCnt))

View File

@ -65,7 +65,8 @@ func (o *Operation) write(target io.Writer, b []byte) (int, error) {
n, err = target.Write(b)
// Adjust the prompt start position by b
rout := runes.ColorFilter([]rune(string(b[:])))
sp := SplitByLine(rout, []rune{}, o.buf.ppos, o.buf.width, 1)
tWidth, _ := o.t.GetWidthHeight()
sp := SplitByLine(rout, []rune{}, o.buf.ppos, tWidth, 1)
if len(sp) > 1 {
o.buf.ppos = len(sp[len(sp)-1])
} else {
@ -83,24 +84,18 @@ func (o *Operation) write(target io.Writer, b []byte) (int, error) {
}
func NewOperation(t *Terminal, cfg *Config) *Operation {
width, height := cfg.FuncGetSize()
op := &Operation{
t: t,
buf: NewRuneBuffer(t, cfg.Prompt, cfg, width, height),
buf: NewRuneBuffer(t, cfg.Prompt, cfg),
outchan: make(chan []rune),
errchan: make(chan error, 1),
}
op.w = op.buf.w
op.SetConfig(cfg)
op.opVim = newVimMode(op)
op.opCompleter = newOpCompleter(op.buf.w, op, width, height)
op.opCompleter = newOpCompleter(op.buf.w, op)
op.opPassword = newOpPassword(op)
op.cfg.FuncOnWidthChanged(func() {
newWidth, newHeight := cfg.FuncGetSize()
op.opCompleter.OnSizeChange(newWidth, newHeight)
op.opSearch.OnSizeChange(newWidth, newHeight)
op.buf.OnSizeChange(newWidth, newHeight)
})
op.cfg.FuncOnWidthChanged(t.OnSizeChange)
go op.ioloop()
return op
}
@ -431,7 +426,7 @@ func (o *Operation) Runes() ([]rune, error) {
// maybe existing text on the same line that ideally we don't
// want to overwrite and cause prompt to jump left. Note that
// this is not perfect but works the majority of the time.
o.buf.getAndSetOffset(o.t)
o.buf.getAndSetOffset()
o.buf.Print() // print prompt & buffer contents
o.t.KickRead()
@ -525,12 +520,11 @@ func (op *Operation) SetConfig(cfg *Config) (*Config, error) {
op.SetPrompt(cfg.Prompt)
op.SetMaskRune(cfg.MaskRune)
op.buf.SetConfig(cfg)
width, height := op.cfg.FuncGetSize()
if cfg.opHistory == nil {
op.SetHistoryPath(cfg.HistoryFile)
cfg.opHistory = op.history
cfg.opSearch = newOpSearch(op.buf.w, op.buf, op.history, cfg, width, height)
cfg.opSearch = newOpSearch(op.buf.w, op.buf, op.history, cfg)
}
op.history = cfg.opHistory
@ -538,8 +532,8 @@ func (op *Operation) SetConfig(cfg *Config) (*Config, error) {
// so if we use it next time, we need to reopen it by `InitHistory()`
op.history.Init()
if op.cfg.AutoComplete != nil {
op.opCompleter = newOpCompleter(op.buf.w, op, width, height)
if op.cfg.AutoComplete != nil && op.opCompleter == nil {
op.opCompleter = newOpCompleter(op.buf.w, op)
}
op.opSearch = cfg.opSearch

View File

@ -18,14 +18,11 @@ type RuneBuffer struct {
buf []rune
idx int
prompt []rune
w io.Writer
w *Terminal
interactive bool
cfg *Config
width int
height int
bck *runeBufferBck
offset string // is offset useful? scrolling means row varies
@ -40,19 +37,6 @@ func (r *RuneBuffer) pushKill(text []rune) {
r.lastKill = append([]rune{}, text...)
}
func (r *RuneBuffer) OnWidthChange(newWidth int) {
r.Lock()
r.width = newWidth
r.Unlock()
}
func (r *RuneBuffer) OnSizeChange(newWidth, newHeight int) {
r.Lock()
r.width = newWidth
r.height = newHeight
r.Unlock()
}
func (r *RuneBuffer) Backup() {
r.Lock()
r.bck = &runeBufferBck{r.buf, r.idx}
@ -69,13 +53,11 @@ func (r *RuneBuffer) Restore() {
})
}
func NewRuneBuffer(w io.Writer, prompt string, cfg *Config, width int, height int) *RuneBuffer {
func NewRuneBuffer(w *Terminal, prompt string, cfg *Config) *RuneBuffer {
rb := &RuneBuffer{
w: w,
interactive: cfg.useInteractive(),
cfg: cfg,
width: width,
height: height,
}
rb.SetPrompt(prompt)
return rb
@ -102,9 +84,8 @@ func (r *RuneBuffer) CurrentWidth(x int) int {
func (r *RuneBuffer) PromptLen() int {
r.Lock()
width := r.promptLen()
r.Unlock()
return width
defer r.Unlock()
return r.promptLen()
}
func (r *RuneBuffer) promptLen() int {
@ -448,12 +429,13 @@ func (r *RuneBuffer) isInLineEdge() bool {
}
func (r *RuneBuffer) getSplitByLine(rs []rune, nextWidth int) [][]rune {
tWidth, _ := r.w.GetWidthHeight()
if r.cfg.EnableMask {
w := runes.Width(r.cfg.MaskRune)
masked := []rune(strings.Repeat(string(r.cfg.MaskRune), len(rs)))
return SplitByLine(runes.ColorFilter(r.prompt), masked, r.ppos, r.width, w)
return SplitByLine(runes.ColorFilter(r.prompt), masked, r.ppos, tWidth, w)
} else {
return SplitByLine(runes.ColorFilter(r.prompt), rs, r.ppos, r.width, nextWidth)
return SplitByLine(runes.ColorFilter(r.prompt), rs, r.ppos, tWidth, nextWidth)
}
}
@ -476,7 +458,8 @@ func (r *RuneBuffer) idxLine(width int) int {
}
func (r *RuneBuffer) CursorLineCount() int {
return r.LineCount() - r.IdxLine(r.width)
tWidth, _ := r.w.GetWidthHeight()
return r.LineCount() - r.IdxLine(tWidth)
}
func (r *RuneBuffer) Refresh(f func()) {
@ -506,7 +489,7 @@ func (r *RuneBuffer) refresh(f func()) {
// will write the offset back to us via stdin and there may already be
// other data in the stdin buffer ahead of it.
// This function is called at the start of readline each time.
func (r *RuneBuffer) getAndSetOffset(t *Terminal) {
func (r *RuneBuffer) getAndSetOffset() {
if !r.interactive {
return
}
@ -517,7 +500,7 @@ func (r *RuneBuffer) getAndSetOffset(t *Terminal) {
// at the beginning of the next line.
r.w.Write([]byte(" \b"))
}
t.GetOffset(r.SetOffset)
r.w.GetOffset(r.SetOffset)
}
func (r *RuneBuffer) SetOffset(offset string) {
@ -528,8 +511,9 @@ func (r *RuneBuffer) SetOffset(offset string) {
func (r *RuneBuffer) setOffset(offset string) {
r.offset = offset
if _, c, ok := (&escapeKeyPair{attr:offset}).Get2(); ok && c > 0 && c < r.width {
r.ppos = c - 1 // c should be 1..width
tWidth, _ := r.w.GetWidthHeight()
if _, c, ok := (&escapeKeyPair{attr:offset}).Get2(); ok && c > 0 && c < tWidth {
r.ppos = c - 1 // c should be 1..tWidth
} else {
r.ppos = 0
}
@ -703,7 +687,8 @@ func (r *RuneBuffer) SetPrompt(prompt string) {
func (r *RuneBuffer) cleanOutput(w io.Writer, idxLine int) {
buf := bufio.NewWriter(w)
if r.width == 0 {
tWidth, _ := r.w.GetWidthHeight()
if tWidth == 0 {
buf.WriteString(strings.Repeat("\r\b", len(r.buf)+r.promptLen()))
buf.Write([]byte("\033[J"))
} else {
@ -724,7 +709,8 @@ func (r *RuneBuffer) Clean() {
}
func (r *RuneBuffer) clean() {
r.cleanWithIdxLine(r.idxLine(r.width))
tWidth, _ := r.w.GetWidthHeight()
r.cleanWithIdxLine(r.idxLine(tWidth))
}
func (r *RuneBuffer) cleanWithIdxLine(idxLine int) {

View File

@ -4,7 +4,6 @@ import (
"bytes"
"container/list"
"fmt"
"io"
)
const (
@ -22,36 +21,24 @@ type opSearch struct {
state int
dir int
source *list.Element
w io.Writer
w *Terminal
buf *RuneBuffer
data []rune
history *opHistory
cfg *Config
markStart int
markEnd int
width int
height int
}
func newOpSearch(w io.Writer, buf *RuneBuffer, history *opHistory, cfg *Config, width int, height int) *opSearch {
func newOpSearch(w *Terminal, buf *RuneBuffer, history *opHistory, cfg *Config) *opSearch {
return &opSearch{
w: w,
buf: buf,
cfg: cfg,
history: history,
width: width,
height: height,
}
}
func (o *opSearch) OnWidthChange(newWidth int) {
o.width = newWidth
}
func (o *opSearch) OnSizeChange(newWidth, newHeight int) {
o.width = newWidth
o.height = newHeight
}
func (o *opSearch) IsSearchMode() bool {
return o.inMode
}
@ -103,7 +90,8 @@ func (o *opSearch) SearchChar(r rune) {
}
func (o *opSearch) SearchMode(dir int) bool {
if o.width == 0 {
tWidth, _ := o.w.GetWidthHeight()
if tWidth == 0 {
return false
}
alreadyInMode := o.inMode
@ -131,6 +119,7 @@ func (o *opSearch) ExitSearchMode(revert bool) {
}
func (o *opSearch) SearchRefresh(x int) {
tWidth, _ := o.w.GetWidthHeight()
if x == -2 {
o.state = S_STATE_FAILING
} else if x >= 0 {
@ -141,7 +130,7 @@ func (o *opSearch) SearchRefresh(x int) {
}
x = o.buf.CurrentWidth(x)
x += o.buf.PromptLen()
x = x % o.width
x = x % tWidth
if o.markStart > 0 {
o.buf.SetStyle(o.markStart, o.markEnd, "4")

View File

@ -19,7 +19,9 @@ type Terminal struct {
wg sync.WaitGroup
sleeping int32
sizeChan chan string
width int // terminal width
height int // terminal height
sizeChan chan string
}
func NewTerminal(cfg *Config) (*Terminal, error) {
@ -33,6 +35,8 @@ func NewTerminal(cfg *Config) (*Terminal, error) {
stopChan: make(chan struct{}, 1),
sizeChan: make(chan string, 1),
}
// Get and cache the current terminal size.
t.OnSizeChange()
go t.ioloop()
return t, nil
@ -244,3 +248,17 @@ func (t *Terminal) SetConfig(c *Config) error {
t.m.Unlock()
return nil
}
// OnSizeChange gets the current terminal size and caches it
func (t *Terminal) OnSizeChange() {
t.m.Lock()
defer t.m.Unlock()
t.width, t.height = t.cfg.FuncGetSize()
}
// GetWidthHeight returns the cached width, height values from the terminal
func (t *Terminal) GetWidthHeight() (width, height int) {
t.m.Lock()
defer t.m.Unlock()
return t.width, t.height
}