Skip to content

Commit

Permalink
fix: race condition on repaint with alt screen
Browse files Browse the repository at this point in the history
I didn't realise at the time, but the tea.Program and the renderer share
the mutex. This make the code difficult to reason about - it turns out
the program sometimes acquires the lock and then calls the
`setAltScreen` method of the renderer which in turns calls `repaint`.
That causes a deadlock as `repaint` is trying to acquire the lock too.
  • Loading branch information
geodimm authored and muesli committed Jun 7, 2022
1 parent 50a8461 commit 958dc20
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
8 changes: 6 additions & 2 deletions standard_renderer.go
Expand Up @@ -230,9 +230,7 @@ func (r *standardRenderer) write(s string) {
}

func (r *standardRenderer) repaint() {
r.mtx.Lock()
r.lastRender = ""
r.mtx.Unlock()
}

func (r *standardRenderer) altScreen() bool {
Expand Down Expand Up @@ -350,7 +348,9 @@ func (r *standardRenderer) handleMessages(msg Msg) {
case repaintMsg:
// Force a repaint by clearing the render cache as we slide into a
// render.
r.mtx.Lock()
r.repaint()
r.mtx.Unlock()

case WindowSizeMsg:
r.mtx.Lock()
Expand All @@ -363,7 +363,9 @@ func (r *standardRenderer) handleMessages(msg Msg) {

// Force a repaint on the area where the scrollable stuff was in this
// update cycle
r.mtx.Lock()
r.repaint()
r.mtx.Unlock()

case syncScrollAreaMsg:
// Re-render scrolling area
Expand All @@ -372,7 +374,9 @@ func (r *standardRenderer) handleMessages(msg Msg) {
r.insertTop(msg.lines, msg.topBoundary, msg.bottomBoundary)

// Force non-scrolling stuff to repaint in this update cycle
r.mtx.Lock()
r.repaint()
r.mtx.Unlock()

case scrollUpMsg:
r.insertTop(msg.lines, msg.topBoundary, msg.bottomBoundary)
Expand Down
2 changes: 2 additions & 0 deletions tea.go
Expand Up @@ -503,7 +503,9 @@ func (p *Program) StartReturningModel() (Model, error) {
continue

case WindowSizeMsg:
p.mtx.Lock()
p.renderer.repaint()
p.mtx.Unlock()

case enterAltScreenMsg:
p.EnterAltScreen()
Expand Down

0 comments on commit 958dc20

Please sign in to comment.