From bca35a593981ed5bd63e83e08ac8f6974e37b204 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 12 May 2024 20:02:51 +0200 Subject: [PATCH 1/2] Simplify UpdateDiff() interface The callback passed to UpdateDiff() is superfluous: in the synchronous case screen.Redraw() is not needed anyway (since the screen is redrawn at every iteration of the main loop), and in the asynchronous case UpdateDiff() can just call screen.Redraw() directly. --- internal/action/actions.go | 4 +--- internal/buffer/buffer.go | 16 ++++------------ internal/display/bufwindow.go | 13 +------------ 3 files changed, 6 insertions(+), 27 deletions(-) diff --git a/internal/action/actions.go b/internal/action/actions.go index 883f8208..0606d616 100644 --- a/internal/action/actions.go +++ b/internal/action/actions.go @@ -1480,9 +1480,7 @@ func (h *BufPane) HalfPageDown() bool { func (h *BufPane) ToggleDiffGutter() bool { if !h.Buf.Settings["diffgutter"].(bool) { h.Buf.Settings["diffgutter"] = true - h.Buf.UpdateDiff(func(synchronous bool) { - screen.Redraw() - }) + h.Buf.UpdateDiff() InfoBar.Message("Enabled diff gutter") } else { h.Buf.Settings["diffgutter"] = false diff --git a/internal/buffer/buffer.go b/internal/buffer/buffer.go index e76793b4..834c0271 100644 --- a/internal/buffer/buffer.go +++ b/internal/buffer/buffer.go @@ -1320,13 +1320,9 @@ func (b *Buffer) updateDiffSync() { // UpdateDiff computes the diff between the diff base and the buffer content. // The update may be performed synchronously or asynchronously. -// UpdateDiff calls the supplied callback when the update is complete. -// The argument passed to the callback is set to true if and only if -// the update was performed synchronously. // If an asynchronous update is already pending when UpdateDiff is called, -// UpdateDiff does not schedule another update, in which case the callback -// is not called. -func (b *Buffer) UpdateDiff(callback func(bool)) { +// UpdateDiff does not schedule another update. +func (b *Buffer) UpdateDiff() { if b.updateDiffTimer != nil { return } @@ -1338,19 +1334,17 @@ func (b *Buffer) UpdateDiff(callback func(bool)) { if lineCount < 1000 { b.updateDiffSync() - callback(true) } else if lineCount < 30000 { b.updateDiffTimer = time.AfterFunc(500*time.Millisecond, func() { b.updateDiffTimer = nil b.updateDiffSync() - callback(false) + screen.Redraw() }) } else { // Don't compute diffs for very large files b.diffLock.Lock() b.diff = make(map[int]DiffStatus) b.diffLock.Unlock() - callback(true) } } @@ -1362,9 +1356,7 @@ func (b *Buffer) SetDiffBase(diffBase []byte) { } else { b.diffBaseLineCount = strings.Count(string(diffBase), "\n") } - b.UpdateDiff(func(synchronous bool) { - screen.Redraw() - }) + b.UpdateDiff() } // DiffStatus returns the diff status for a line in the buffer diff --git a/internal/display/bufwindow.go b/internal/display/bufwindow.go index 942dd167..4eeafd3b 100644 --- a/internal/display/bufwindow.go +++ b/internal/display/bufwindow.go @@ -384,18 +384,7 @@ func (w *BufWindow) displayBuffer() { if b.ModifiedThisFrame { if b.Settings["diffgutter"].(bool) { - b.UpdateDiff(func(synchronous bool) { - // If the diff was updated asynchronously, the outer call to - // displayBuffer might already be completed and we need to - // schedule a redraw in order to display the new diff. - // Note that this cannot lead to an infinite recursion - // because the modifications were cleared above so there won't - // be another call to UpdateDiff when displayBuffer is called - // during the redraw. - if !synchronous { - screen.Redraw() - } - }) + b.UpdateDiff() } b.ModifiedThisFrame = false } From 5a159ce444f02f8cbb7002d7b7522c2a5b81f505 Mon Sep 17 00:00:00 2001 From: Dmytro Maluka Date: Sun, 12 May 2024 20:35:07 +0200 Subject: [PATCH 2/2] updateDiffSync(): fix potential race When updateDiffSync() is called asynchronously, it should lock the line array when calling Bytes(), to prevent race if the line array is being modified by the main goroutine in the meantime. --- internal/buffer/buffer.go | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/internal/buffer/buffer.go b/internal/buffer/buffer.go index 834c0271..144ccafc 100644 --- a/internal/buffer/buffer.go +++ b/internal/buffer/buffer.go @@ -1280,7 +1280,7 @@ func (b *Buffer) Write(bytes []byte) (n int, err error) { return len(bytes), nil } -func (b *Buffer) updateDiffSync() { +func (b *Buffer) updateDiff(synchronous bool) { b.diffLock.Lock() defer b.diffLock.Unlock() @@ -1291,7 +1291,16 @@ func (b *Buffer) updateDiffSync() { } differ := dmp.New() - baseRunes, bufferRunes, _ := differ.DiffLinesToRunes(string(b.diffBase), string(b.Bytes())) + + if !synchronous { + b.Lock() + } + bytes := b.Bytes() + if !synchronous { + b.Unlock() + } + + baseRunes, bufferRunes, _ := differ.DiffLinesToRunes(string(b.diffBase), string(bytes)) diffs := differ.DiffMainRunes(baseRunes, bufferRunes, false) lineN := 0 @@ -1333,11 +1342,11 @@ func (b *Buffer) UpdateDiff() { } if lineCount < 1000 { - b.updateDiffSync() + b.updateDiff(true) } else if lineCount < 30000 { b.updateDiffTimer = time.AfterFunc(500*time.Millisecond, func() { b.updateDiffTimer = nil - b.updateDiffSync() + b.updateDiff(false) screen.Redraw() }) } else {