mirror of
https://github.com/jesseduffield/lazygit.git
synced 2026-09-10 07:36:27 -04:00
Synchronize async view rendering (#5791)
Several fixes for the last concurrency problems we still had: we used to access view properties on worker threads, which isn't allowed (writing to a view itself is fine and guarded by a mutex, but other fields are not, and even that mutex had a few holes that are plugged here). With this, our integration test suite seems to be fully deterministic and race-free, so we also remove the retry safety net (MaxAttempts=2) that we were using to address flakiness.
This commit is contained in:
Stefan Haller
GitHub
commit
4cf12a5b7b
|
|
@ -6,6 +6,7 @@ import (
|
|||
|
||||
"github.com/jesseduffield/generics/maps"
|
||||
"github.com/samber/lo"
|
||||
"github.com/sasha-s/go-deadlock"
|
||||
"github.com/sirupsen/logrus"
|
||||
)
|
||||
|
||||
|
|
@ -50,6 +51,13 @@ type PatchBuilder struct {
|
|||
fileInfoMap map[string]*fileInfo
|
||||
Log *logrus.Entry
|
||||
|
||||
// mutex guards the fields that a git worker can mutate (via Reset, at the
|
||||
// end of a patch-consuming operation) while the UI thread reads them to
|
||||
// render — chiefly To and the fileInfoMap pointer. The map's *entries* are
|
||||
// only ever touched on the UI thread, so we only hold the lock long enough
|
||||
// to read or swap the fields, never across the git I/O in getFileInfo.
|
||||
mutex deadlock.Mutex
|
||||
|
||||
// loadFileDiff loads the diff of a file, for a given to (typically a commit hash)
|
||||
loadFileDiff loadFileDiffFunc
|
||||
}
|
||||
|
|
@ -62,6 +70,9 @@ func NewPatchBuilder(log *logrus.Entry, loadFileDiff loadFileDiffFunc) *PatchBui
|
|||
}
|
||||
|
||||
func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) {
|
||||
p.mutex.Lock()
|
||||
defer p.mutex.Unlock()
|
||||
|
||||
p.To = to
|
||||
p.From = from
|
||||
p.reverse = reverse
|
||||
|
|
@ -69,10 +80,21 @@ func (p *PatchBuilder) Start(from, to string, reverse bool, canRebase bool) {
|
|||
p.fileInfoMap = map[string]*fileInfo{}
|
||||
}
|
||||
|
||||
// snapshotFileInfoMap returns the current fileInfoMap under the lock. The map's
|
||||
// entries are only mutated on the UI thread, so callers can read the returned
|
||||
// map without holding the lock; the lock only serializes the pointer swap that
|
||||
// Reset/Start do (potentially from a git worker) against these reads.
|
||||
func (p *PatchBuilder) snapshotFileInfoMap() map[string]*fileInfo {
|
||||
p.mutex.Lock()
|
||||
defer p.mutex.Unlock()
|
||||
|
||||
return p.fileInfoMap
|
||||
}
|
||||
|
||||
func (p *PatchBuilder) PatchToApply(reverse bool, turnAddedFilesIntoDiffAgainstEmptyFile bool) string {
|
||||
var patch strings.Builder
|
||||
|
||||
for filename, info := range p.fileInfoMap {
|
||||
for filename, info := range p.snapshotFileInfoMap() {
|
||||
if info.mode == UNSELECTED {
|
||||
continue
|
||||
}
|
||||
|
|
@ -130,12 +152,17 @@ func (p *PatchBuilder) RemoveFile(filename string, previousPath string) error {
|
|||
}
|
||||
|
||||
func (p *PatchBuilder) getFileInfo(filename string, previousPath string) (*fileInfo, error) {
|
||||
info, ok := p.fileInfoMap[filename]
|
||||
p.mutex.Lock()
|
||||
fileInfoMap := p.fileInfoMap
|
||||
from, to, reverse := p.From, p.To, p.reverse
|
||||
p.mutex.Unlock()
|
||||
|
||||
info, ok := fileInfoMap[filename]
|
||||
if ok {
|
||||
return info, nil
|
||||
}
|
||||
|
||||
diff, err := p.loadFileDiff(p.From, p.To, p.reverse, filename, previousPath, true)
|
||||
diff, err := p.loadFileDiff(from, to, reverse, filename, previousPath, true)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
|
|
@ -145,7 +172,7 @@ func (p *PatchBuilder) getFileInfo(filename string, previousPath string) (*fileI
|
|||
previousPath: previousPath,
|
||||
}
|
||||
|
||||
p.fileInfoMap[filename] = info
|
||||
fileInfoMap[filename] = info
|
||||
|
||||
return info, nil
|
||||
}
|
||||
|
|
@ -220,14 +247,16 @@ func (p *PatchBuilder) RenderPatchForFile(opts RenderPatchForFileOpts) string {
|
|||
}
|
||||
|
||||
func (p *PatchBuilder) renderEachFilePatch(plain bool) []string {
|
||||
fileInfoMap := p.snapshotFileInfoMap()
|
||||
|
||||
// sort files by name then iterate through and render each patch
|
||||
filenames := maps.Keys(p.fileInfoMap)
|
||||
filenames := maps.Keys(fileInfoMap)
|
||||
|
||||
sort.Strings(filenames)
|
||||
patches := lo.Map(filenames, func(filename string, _ int) string {
|
||||
return p.RenderPatchForFile(RenderPatchForFileOpts{
|
||||
Filename: filename,
|
||||
PreviousPath: p.fileInfoMap[filename].previousPath,
|
||||
PreviousPath: fileInfoMap[filename].previousPath,
|
||||
Plain: plain,
|
||||
Reverse: false,
|
||||
TurnAddedFilesIntoDiffAgainstEmptyFile: true,
|
||||
|
|
@ -245,11 +274,16 @@ func (p *PatchBuilder) RenderAggregatedPatch(plain bool) string {
|
|||
}
|
||||
|
||||
func (p *PatchBuilder) GetFileStatus(filename string, parent string) PatchStatus {
|
||||
if parent != p.To {
|
||||
p.mutex.Lock()
|
||||
to := p.To
|
||||
fileInfoMap := p.fileInfoMap
|
||||
p.mutex.Unlock()
|
||||
|
||||
if parent != to {
|
||||
return UNSELECTED
|
||||
}
|
||||
|
||||
info, ok := p.fileInfoMap[filename]
|
||||
info, ok := fileInfoMap[filename]
|
||||
if !ok {
|
||||
return UNSELECTED
|
||||
}
|
||||
|
|
@ -267,16 +301,22 @@ func (p *PatchBuilder) GetFileIncLineIndices(filename string, previousPath strin
|
|||
|
||||
// clears the patch
|
||||
func (p *PatchBuilder) Reset() {
|
||||
p.mutex.Lock()
|
||||
defer p.mutex.Unlock()
|
||||
|
||||
p.To = ""
|
||||
p.fileInfoMap = map[string]*fileInfo{}
|
||||
}
|
||||
|
||||
func (p *PatchBuilder) Active() bool {
|
||||
p.mutex.Lock()
|
||||
defer p.mutex.Unlock()
|
||||
|
||||
return p.To != ""
|
||||
}
|
||||
|
||||
func (p *PatchBuilder) IsEmpty() bool {
|
||||
for _, fileInfo := range p.fileInfoMap {
|
||||
for _, fileInfo := range p.snapshotFileInfoMap() {
|
||||
if fileInfo.mode == WHOLE || (fileInfo.mode == PART && len(fileInfo.includedLineIndices) > 0) {
|
||||
return false
|
||||
}
|
||||
|
|
@ -287,9 +327,12 @@ func (p *PatchBuilder) IsEmpty() bool {
|
|||
|
||||
// if any of these things change we'll need to reset and start a new patch
|
||||
func (p *PatchBuilder) NewPatchRequired(from string, to string, reverse bool) bool {
|
||||
p.mutex.Lock()
|
||||
defer p.mutex.Unlock()
|
||||
|
||||
return from != p.From || to != p.To || reverse != p.reverse
|
||||
}
|
||||
|
||||
func (p *PatchBuilder) AllFilesInPatch() []string {
|
||||
return lo.Keys(p.fileInfoMap)
|
||||
return lo.Keys(p.snapshotFileInfoMap())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,6 +34,14 @@ type escapeInterpreter struct {
|
|||
// modelled — we don't track the col argument of CUPs, and most
|
||||
// pager-style emitters use col 1 anyway.
|
||||
screenRow, screenCol int
|
||||
|
||||
// The screen width that soft-wraps are counted against (see
|
||||
// notifyCellsWritten). It's a snapshot of the view's InnerWidth taken on
|
||||
// the UI thread (in NewView, and refreshed per render via
|
||||
// View.SetContentWidth), rather than read live from the view's dimensions:
|
||||
// a view's output is written from a task goroutine, and reading the live
|
||||
// dimensions there would race the UI thread updating them during layout.
|
||||
screenColMax int
|
||||
}
|
||||
|
||||
type (
|
||||
|
|
@ -175,8 +183,8 @@ func (ei *escapeInterpreter) notifyColumnReset() {
|
|||
// columns; if that crosses the right edge of a `screenColMax`-wide pty
|
||||
// screen, the corresponding number of soft-wraps are added to screenRow
|
||||
// so subsequent CUPs land on the right line.
|
||||
func (ei *escapeInterpreter) notifyCellsWritten(width, screenColMax int) {
|
||||
if screenColMax <= 0 {
|
||||
func (ei *escapeInterpreter) notifyCellsWritten(width int) {
|
||||
if ei.screenColMax <= 0 {
|
||||
return
|
||||
}
|
||||
// One column at a time: matches ConPTY's "pending wrap" semantics
|
||||
|
|
@ -185,7 +193,7 @@ func (ei *escapeInterpreter) notifyCellsWritten(width, screenColMax int) {
|
|||
// columns rather than doing the math in one shot so wide cells on a
|
||||
// row boundary still wrap cleanly.
|
||||
for range width {
|
||||
if ei.screenCol > screenColMax {
|
||||
if ei.screenCol > ei.screenColMax {
|
||||
ei.screenRow++
|
||||
ei.screenCol = 1
|
||||
}
|
||||
|
|
|
|||
|
|
@ -148,6 +148,10 @@ type Gui struct {
|
|||
maxX, maxY int
|
||||
outputMode OutputMode
|
||||
stop chan struct{}
|
||||
// loopExited is closed when MainLoop returns, so callers (e.g. the
|
||||
// integration-test harness) can wait for the event loop to actually finish
|
||||
// rather than polling or sleeping a fixed interval.
|
||||
loopExited chan struct{}
|
||||
|
||||
// BgColor and FgColor allow to configure the background and foreground
|
||||
// colors of the GUI.
|
||||
|
|
@ -260,6 +264,7 @@ func NewGui(opts NewGuiOpts) (*Gui, error) {
|
|||
g.outputMode = opts.OutputMode
|
||||
|
||||
g.stop = make(chan struct{})
|
||||
g.loopExited = make(chan struct{})
|
||||
|
||||
g.gEvents = make(chan GocuiEvent, 20)
|
||||
g.userEvents = newUserEventQueue()
|
||||
|
|
@ -288,6 +293,12 @@ func NewGui(opts NewGuiOpts) (*Gui, error) {
|
|||
|
||||
g.playRecording = opts.PlayRecording
|
||||
|
||||
// Record the UI thread here, at construction. This assumes NewGui is called
|
||||
// on the same goroutine that will run MainLoop, which holds for all our
|
||||
// callers -- and it means IsUIThread is already correct for the UI work that
|
||||
// runs during startup, before we reach MainLoop.
|
||||
g.uiThreadID.Store(goid.Get())
|
||||
|
||||
return g, nil
|
||||
}
|
||||
|
||||
|
|
@ -348,6 +359,11 @@ func (g *Gui) Close() {
|
|||
Screen.Fini()
|
||||
}
|
||||
|
||||
// LoopExited returns a channel that is closed once MainLoop has returned.
|
||||
func (g *Gui) LoopExited() <-chan struct{} {
|
||||
return g.loopExited
|
||||
}
|
||||
|
||||
// Size returns the terminal's size.
|
||||
func (g *Gui) Size() (x, y int) {
|
||||
return g.maxX, g.maxY
|
||||
|
|
@ -385,7 +401,7 @@ func (g *Gui) SetView(name string, x0, y0, x1, y1 int, overlaps byte) (*View, er
|
|||
v.y1 = y1
|
||||
|
||||
if sizeChanged {
|
||||
v.clearViewLines()
|
||||
v.ClearViewLines()
|
||||
|
||||
if v.Editable {
|
||||
cursorX, cursorY := v.TextArea.GetCursorXY()
|
||||
|
|
@ -965,7 +981,7 @@ func (g *Gui) SetManagerFunc(manager func(*Gui) error) {
|
|||
// MainLoop runs the main loop until an error is returned. A successful
|
||||
// finish should return ErrQuit.
|
||||
func (g *Gui) MainLoop() error {
|
||||
g.uiThreadID.Store(goid.Get())
|
||||
defer close(g.loopExited)
|
||||
|
||||
go func() {
|
||||
for {
|
||||
|
|
@ -1461,7 +1477,7 @@ func (g *Gui) flush() error {
|
|||
// if GUI's size has changed, we need to redraw all views
|
||||
if maxX != g.maxX || maxY != g.maxY {
|
||||
for _, v := range g.views {
|
||||
v.clearViewLines()
|
||||
v.ClearViewLines()
|
||||
}
|
||||
}
|
||||
g.maxX, g.maxY = maxX, maxY
|
||||
|
|
@ -1500,7 +1516,7 @@ func viewsToRedrawContentOnly(views []*View) []*View {
|
|||
redrawIndexes := set.New[int]()
|
||||
|
||||
for i, v := range views {
|
||||
if !v.tainted && !redrawIndexes.Includes(i) {
|
||||
if !v.IsTainted() && !redrawIndexes.Includes(i) {
|
||||
continue
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ package gocui
|
|||
import (
|
||||
"fmt"
|
||||
"io"
|
||||
"slices"
|
||||
"strings"
|
||||
"sync"
|
||||
"unicode"
|
||||
|
|
@ -214,6 +215,16 @@ func (v *View) clearViewLines() {
|
|||
v.clearHover()
|
||||
}
|
||||
|
||||
// ClearViewLines is clearViewLines guarded by writeMutex. It's for callers on
|
||||
// the UI thread (the layout pass) that touch a view whose content a task
|
||||
// goroutine may be writing concurrently: viewLines/tainted/hover are all
|
||||
// buffer state that writeMutex protects.
|
||||
func (v *View) ClearViewLines() {
|
||||
v.writeMutex.Lock()
|
||||
defer v.writeMutex.Unlock()
|
||||
v.clearViewLines()
|
||||
}
|
||||
|
||||
type searcher struct {
|
||||
searchString string
|
||||
searchPositions []SearchPosition
|
||||
|
|
@ -536,9 +547,20 @@ func NewView(name string, x0, y0, x1, y1 int, mode OutputMode) *View {
|
|||
v.SelFgColor, v.SelBgColor = ColorDefault, ColorDefault
|
||||
v.InactiveViewSelBgColor = ColorDefault
|
||||
v.TitleColor, v.FrameColor = ColorDefault, ColorDefault
|
||||
v.ei.screenColMax = v.InnerWidth()
|
||||
return v
|
||||
}
|
||||
|
||||
// SetContentWidth tells the view the screen width that content written to it
|
||||
// should count soft-wraps against (see escapeInterpreter.notifyCellsWritten).
|
||||
// Callers pass the view's InnerWidth; it's a separate call, made on the UI
|
||||
// thread when a render starts, so that the task goroutine that streams the
|
||||
// content can consult this snapshot instead of reading the view's live
|
||||
// dimensions (which the UI thread mutates during layout).
|
||||
func (v *View) SetContentWidth(width int) {
|
||||
v.ei.screenColMax = width
|
||||
}
|
||||
|
||||
// Dimensions returns the dimensions of the View
|
||||
func (v *View) Dimensions() (int, int, int, int) {
|
||||
return v.x0, v.y0, v.x1, v.y1
|
||||
|
|
@ -907,7 +929,7 @@ func (v *View) write(p []byte) {
|
|||
for _, c := range cells {
|
||||
totalWidth += c.width
|
||||
}
|
||||
v.ei.notifyCellsWritten(totalWidth, v.InnerWidth())
|
||||
v.ei.notifyCellsWritten(totalWidth)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1125,10 +1147,25 @@ func (v *View) CopyContent(from *View) {
|
|||
v.writeMutex.Lock()
|
||||
defer v.writeMutex.Unlock()
|
||||
|
||||
// A background task may be streaming output into the source view's buffer
|
||||
// via Write, so read it under its own lock. The source is always a
|
||||
// different view than the destination (see the sole caller,
|
||||
// moveMainContextToTop), and no other code holds two view write locks at
|
||||
// once, so this can't deadlock.
|
||||
from.writeMutex.Lock()
|
||||
defer from.writeMutex.Unlock()
|
||||
|
||||
v.clear()
|
||||
|
||||
v.lines = from.lines
|
||||
v.viewLines = from.viewLines
|
||||
// Clone the row slices rather than sharing them: the source view stays
|
||||
// live (its streaming task keeps appending rows, and refreshViewLinesIfNeeded
|
||||
// fills each row's wrapping cache in place via &lines[i]), so sharing the
|
||||
// backing arrays would race those writes against this view's own rendering.
|
||||
// This is a shallow clone -- the per-row cell data is immutable once written
|
||||
// and stays shared, so the cost is proportional to the number of rows, not
|
||||
// their contents.
|
||||
v.lines = slices.Clone(from.lines)
|
||||
v.viewLines = slices.Clone(from.viewLines)
|
||||
v.ox = from.ox
|
||||
v.oy = from.oy
|
||||
v.cx = from.cx
|
||||
|
|
@ -1276,6 +1313,8 @@ func (v *View) updateSearchPositions() {
|
|||
|
||||
// IsTainted tells us if the view is tainted
|
||||
func (v *View) IsTainted() bool {
|
||||
v.writeMutex.Lock()
|
||||
defer v.writeMutex.Unlock()
|
||||
return v.tainted
|
||||
}
|
||||
|
||||
|
|
@ -1524,6 +1563,9 @@ func (v *View) BufferLines() []string {
|
|||
// Buffer returns a string with the contents of the view's internal
|
||||
// buffer.
|
||||
func (v *View) Buffer() string {
|
||||
v.writeMutex.Lock()
|
||||
defer v.writeMutex.Unlock()
|
||||
|
||||
return linesToString(v.lines)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -81,10 +81,17 @@ func (self *SuggestionsContext) SetSuggestions(suggestions []*types.Suggestion)
|
|||
}
|
||||
|
||||
func (self *SuggestionsContext) RefreshSuggestions() {
|
||||
// Capture the suggestions function and the prompt input here, on the UI
|
||||
// thread, rather than inside the worker below: the main thread rewrites both
|
||||
// (State.FindSuggestions and the prompt's TextArea) when it (re)creates a
|
||||
// prompt panel, so reading them from the worker races those writes. It's
|
||||
// also more correct -- we search for the input as it was when dispatched,
|
||||
// which is what this request's AsyncHandler id corresponds to.
|
||||
findSuggestionsFn := self.State.FindSuggestions
|
||||
promptInput := self.c.GetPromptInput()
|
||||
self.State.AsyncHandler.Do(func() func() {
|
||||
findSuggestionsFn := self.State.FindSuggestions
|
||||
if findSuggestionsFn != nil {
|
||||
suggestions := findSuggestionsFn(self.c.GetPromptInput())
|
||||
suggestions := findSuggestionsFn(promptInput)
|
||||
return func() { self.SetSuggestions(suggestions) }
|
||||
}
|
||||
return func() {}
|
||||
|
|
|
|||
|
|
@ -77,9 +77,7 @@ func (self *ConfirmationHelper) wrappedPromptConfirmationFunction(
|
|||
}
|
||||
|
||||
func (self *ConfirmationHelper) DeactivateConfirmation() {
|
||||
self.c.Mutexes().PopupMutex.Lock()
|
||||
self.c.State().GetRepoState().SetCurrentPopupOpts(nil)
|
||||
self.c.Mutexes().PopupMutex.Unlock()
|
||||
|
||||
self.c.Views().Confirmation.Visible = false
|
||||
|
||||
|
|
@ -87,9 +85,7 @@ func (self *ConfirmationHelper) DeactivateConfirmation() {
|
|||
}
|
||||
|
||||
func (self *ConfirmationHelper) DeactivatePrompt() {
|
||||
self.c.Mutexes().PopupMutex.Lock()
|
||||
self.c.State().GetRepoState().SetCurrentPopupOpts(nil)
|
||||
self.c.Mutexes().PopupMutex.Unlock()
|
||||
|
||||
self.c.Views().Prompt.Visible = false
|
||||
self.c.Views().Suggestions.Visible = false
|
||||
|
|
@ -188,9 +184,6 @@ func characterForMask(mask bool) string {
|
|||
}
|
||||
|
||||
func (self *ConfirmationHelper) CreatePopupPanel(ctx goContext.Context, opts types.CreatePopupPanelOpts) {
|
||||
self.c.Mutexes().PopupMutex.Lock()
|
||||
defer self.c.Mutexes().PopupMutex.Unlock()
|
||||
|
||||
_, cancel := goContext.WithCancel(ctx)
|
||||
|
||||
// we don't allow interruptions of non-loader popups in case we get stuck somehow
|
||||
|
|
|
|||
|
|
@ -127,7 +127,7 @@ func (self *MergeAndRebaseHelper) genericMergeCommandImpl(command string, showWa
|
|||
needsSubprocess := (effectiveStatus == models.WORKING_TREE_STATE_MERGING && command != REBASE_OPTION_ABORT && self.c.UserConfig().Git.Merging.ManualCommit) ||
|
||||
// but we'll also use a subprocess if we have exec todos; those are likely to be lengthy build
|
||||
// tasks whose output the user will want to see in the terminal
|
||||
(effectiveStatus == models.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos())
|
||||
(effectiveStatus == models.WORKING_TREE_STATE_REBASING && command != REBASE_OPTION_ABORT && self.hasExecTodos(calledFromWorker))
|
||||
|
||||
if needsSubprocess {
|
||||
// TODO: see if we should be calling more of the code from self.Git.Rebase.GenericMergeOrRebaseAction
|
||||
|
|
@ -168,16 +168,31 @@ func commitSelectionAfterMerge(createdNewCommit bool) types.CommitSelectionBehav
|
|||
return types.KeepCommitSelectionByHash
|
||||
}
|
||||
|
||||
func (self *MergeAndRebaseHelper) hasExecTodos() bool {
|
||||
for _, commit := range self.c.Model().Commits {
|
||||
if !commit.IsTODO() {
|
||||
break
|
||||
}
|
||||
if commit.Action == todo.Exec {
|
||||
return true
|
||||
func (self *MergeAndRebaseHelper) hasExecTodos(calledFromWorker bool) bool {
|
||||
check := func() bool {
|
||||
for _, commit := range self.c.Model().Commits {
|
||||
if !commit.IsTODO() {
|
||||
break
|
||||
}
|
||||
if commit.Action == todo.Exec {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
return false
|
||||
|
||||
// This reads the model, which is only safe on the UI thread, so bounce there
|
||||
// when we're being called from a worker.
|
||||
if !calledFromWorker {
|
||||
return check()
|
||||
}
|
||||
|
||||
result := false
|
||||
_ = self.c.GocuiGui().OnUIThreadAndWait(func() error {
|
||||
result = check()
|
||||
return nil
|
||||
})
|
||||
return result
|
||||
}
|
||||
|
||||
var conflictStrings = []string{
|
||||
|
|
|
|||
|
|
@ -51,32 +51,28 @@ func (self *MergeConflictsHelper) resetMergeState() {
|
|||
self.context().GetState().Reset()
|
||||
}
|
||||
|
||||
func (self *MergeConflictsHelper) EscapeMerge(background bool) error {
|
||||
self.resetMergeState()
|
||||
// EscapeMerge returns from the merge conflicts view to the files context. It
|
||||
// must be called on the UI thread, without the merge-conflicts mutex held:
|
||||
// pushing the files context renders the newly focused file to the main view,
|
||||
// which can take the mutex again (via SetMergeState).
|
||||
func (self *MergeConflictsHelper) EscapeMerge() {
|
||||
self.ResetMergeState()
|
||||
|
||||
// doing this in separate UI thread so that we're not still holding the lock by the time refresh the file
|
||||
onUIThread := self.c.OnUIThread
|
||||
if background {
|
||||
// Reached from a background files refresh; keep it off the busy count
|
||||
// (see the *Background dispatch methods) so it doesn't block a repo switch.
|
||||
onUIThread = self.c.OnUIThreadBackground
|
||||
// The files refresh may already have opened the prompt to continue the
|
||||
// rebase/merge on top of us (if all conflicts are resolved); in that case
|
||||
// don't push the files context over it.
|
||||
if self.c.Context().IsCurrent(self.c.Contexts().MergeConflicts) {
|
||||
self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{})
|
||||
}
|
||||
onUIThread(func() error {
|
||||
// There is a race condition here: refreshing the files scope can trigger the
|
||||
// confirmation context to be pushed if all conflicts are resolved (prompting
|
||||
// to continue the merge/rebase. In that case, we don't want to then push the
|
||||
// files context over it.
|
||||
// So long as both places call OnUIThread, we're fine.
|
||||
if self.c.Context().IsCurrent(self.c.Contexts().MergeConflicts) {
|
||||
self.c.Context().Push(self.c.Contexts().Files, types.OnFocusOpts{})
|
||||
}
|
||||
return nil
|
||||
})
|
||||
return nil
|
||||
}
|
||||
|
||||
func (self *MergeConflictsHelper) SetConflictsAndRender(path string) (bool, error) {
|
||||
hasConflicts, err := self.setMergeStateWithoutLock(path)
|
||||
// SetConflictsAndRender re-reads the file being merged and re-renders the
|
||||
// merge conflicts view. Returns whether the file still has conflicts.
|
||||
func (self *MergeConflictsHelper) SetConflictsAndRender() (bool, error) {
|
||||
self.context().GetMutex().Lock()
|
||||
defer self.context().GetMutex().Unlock()
|
||||
|
||||
hasConflicts, err := self.setMergeStateWithoutLock(self.context().GetState().GetPath())
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
|
|
@ -126,21 +122,18 @@ func (self *MergeConflictsHelper) Render() {
|
|||
})
|
||||
}
|
||||
|
||||
func (self *MergeConflictsHelper) RefreshMergeState(background bool) error {
|
||||
self.c.Contexts().MergeConflicts.GetMutex().Lock()
|
||||
defer self.c.Contexts().MergeConflicts.GetMutex().Unlock()
|
||||
|
||||
func (self *MergeConflictsHelper) RefreshMergeState() error {
|
||||
if self.c.Context().Current().GetKey() != context.MERGE_CONFLICTS_CONTEXT_KEY {
|
||||
return nil
|
||||
}
|
||||
|
||||
hasConflicts, err := self.SetConflictsAndRender(self.c.Contexts().MergeConflicts.GetState().GetPath())
|
||||
hasConflicts, err := self.SetConflictsAndRender()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
if !hasConflicts {
|
||||
return self.EscapeMerge(background)
|
||||
self.EscapeMerge()
|
||||
}
|
||||
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -398,11 +398,28 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr
|
|||
}
|
||||
|
||||
if scopeSet.Includes(types.PATCH_BUILDING) {
|
||||
refresh("patch building", func() { self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{}) })
|
||||
refresh("patch building", func() {
|
||||
// Bounce onto the UI thread, like the staging panel above:
|
||||
// RefreshPatchBuildingPanel reads the commit-files selection and
|
||||
// sets the patch view's origin, neither of which may run off the UI
|
||||
// thread. Guard on the generation so a repo switch mid-refresh drops
|
||||
// it, like the model bounces.
|
||||
self.onUIThreadUnlessRepoChanged(env, func() {
|
||||
self.patchBuildingHelper.RefreshPatchBuildingPanel(types.OnFocusOpts{})
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
if scopeSet.Includes(types.MERGE_CONFLICTS) {
|
||||
refresh("merge conflicts", func() { _ = self.mergeConflictsHelper.RefreshMergeState(env.background) })
|
||||
refresh("merge conflicts", func() {
|
||||
// Bounce onto the UI thread, like the staging and patch-building
|
||||
// panels above: RefreshMergeState reads the current context and
|
||||
// renders (or escapes) the merge-conflicts view, none of which may
|
||||
// run off the UI thread.
|
||||
self.onUIThreadUnlessRepoChanged(env, func() {
|
||||
_ = self.mergeConflictsHelper.RefreshMergeState()
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
self.refreshStatus(env)
|
||||
|
|
@ -1304,10 +1321,6 @@ func (self *RefreshHelper) refreshStateFiles(captured capturedFilesState, env re
|
|||
// that a subsequent branches refresh can use them for recency sorting without
|
||||
// having to read them back out of the model.
|
||||
func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, env refreshEnv, selectTopEntry bool) ([]*models.Commit, error) {
|
||||
// pulling state into its own variable in case it gets swapped out for another state
|
||||
// and we get an out of bounds exception
|
||||
model := self.c.Model()
|
||||
|
||||
// load does the git work on the worker and returns the new value for a
|
||||
// reflog slice, reading the existing slice (captured on the UI thread) for
|
||||
// the incremental fetch. The caller writes the result in the bounce.
|
||||
|
|
@ -1343,8 +1356,8 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en
|
|||
}
|
||||
|
||||
self.onUIThreadUnlessRepoChanged(env, func() {
|
||||
model.ReflogCommits = reflogCommits
|
||||
model.FilteredReflogCommits = filteredReflogCommits
|
||||
self.c.Model().ReflogCommits = reflogCommits
|
||||
self.c.Model().FilteredReflogCommits = filteredReflogCommits
|
||||
// Setting the selection here, in the same bounce that writes the list,
|
||||
// keeps it on the UI thread and atomic with the list update. Setting the
|
||||
// selection doesn't scroll the view, so also reset the origin.
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ package helpers
|
|||
import (
|
||||
"fmt"
|
||||
"strings"
|
||||
"sync/atomic"
|
||||
|
||||
"github.com/jesseduffield/generics/set"
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
|
||||
|
|
@ -28,14 +29,20 @@ import (
|
|||
|
||||
type SuggestionsHelper struct {
|
||||
c *HelperCommon
|
||||
|
||||
// filesTrie holds the repo's file paths for file-path suggestions. It's
|
||||
// rebuilt asynchronously and read from the suggestions worker goroutine, so
|
||||
// it lives here as an atomic pointer rather than in the (UI-thread-only)
|
||||
// model.
|
||||
filesTrie atomic.Pointer[patricia.Trie]
|
||||
}
|
||||
|
||||
func NewSuggestionsHelper(
|
||||
c *HelperCommon,
|
||||
) *SuggestionsHelper {
|
||||
return &SuggestionsHelper{
|
||||
c: c,
|
||||
}
|
||||
self := &SuggestionsHelper{c: c}
|
||||
self.filesTrie.Store(patricia.NewTrie())
|
||||
return self
|
||||
}
|
||||
|
||||
func (self *SuggestionsHelper) getRemoteNames() []string {
|
||||
|
|
@ -137,9 +144,9 @@ func (self *SuggestionsHelper) GetFilePathSuggestionsFunc() func(string) []*type
|
|||
trie.Insert(patricia.Prefix(file), file)
|
||||
}
|
||||
|
||||
// cache the trie for future use
|
||||
self.filesTrie.Store(trie)
|
||||
self.c.OnUIThread(func() error {
|
||||
// cache the trie for future use
|
||||
self.c.Model().FilesTrie = trie
|
||||
self.c.Contexts().Suggestions.RefreshSuggestions()
|
||||
return nil
|
||||
})
|
||||
|
|
@ -148,9 +155,10 @@ func (self *SuggestionsHelper) GetFilePathSuggestionsFunc() func(string) []*type
|
|||
})
|
||||
|
||||
return func(input string) []*types.Suggestion {
|
||||
filesTrie := self.filesTrie.Load()
|
||||
matchingNames := []string{}
|
||||
if self.c.UserConfig().Gui.UseFuzzySearch() {
|
||||
_ = self.c.Model().FilesTrie.VisitFuzzy(patricia.Prefix(input), true, func(prefix patricia.Prefix, item patricia.Item, skipped int) error {
|
||||
_ = filesTrie.VisitFuzzy(patricia.Prefix(input), true, func(prefix patricia.Prefix, item patricia.Item, skipped int) error {
|
||||
matchingNames = append(matchingNames, item.(string))
|
||||
return nil
|
||||
})
|
||||
|
|
@ -159,7 +167,7 @@ func (self *SuggestionsHelper) GetFilePathSuggestionsFunc() func(string) []*type
|
|||
matchingNames = utils.FilterStrings(input, matchingNames, true)
|
||||
} else {
|
||||
substrings := strings.Fields(input)
|
||||
_ = self.c.Model().FilesTrie.Visit(func(prefix patricia.Prefix, item patricia.Item) error {
|
||||
_ = filesTrie.Visit(func(prefix patricia.Prefix, item patricia.Item) error {
|
||||
for _, sub := range substrings {
|
||||
if !utils.CaseAwareContains(item.(string), sub) {
|
||||
return nil
|
||||
|
|
|
|||
|
|
@ -170,11 +170,16 @@ func (self *StashController) handleStashDrop(stashEntries []*models.StashEntry)
|
|||
Prompt: self.c.Tr.SureDropStashEntry,
|
||||
HandleConfirm: func() error {
|
||||
self.c.LogAction(self.c.Tr.Actions.DropStash)
|
||||
// Refresh once at the end rather than after each drop: an async
|
||||
// refresh from the UI thread finishes in the background, so firing
|
||||
// one per iteration lets the workers race and an earlier, stale
|
||||
// result can land last. The indices are captured up front and we
|
||||
// drop highest-first, so the remaining lower indices stay valid
|
||||
// without an intervening refresh.
|
||||
defer self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})
|
||||
for i := len(stashEntries) - 1; i >= 0; i-- {
|
||||
self.c.LogCommand(fmt.Sprintf(self.c.Tr.Log.DroppingStash, stashEntries[i].Hash), false)
|
||||
err := self.c.Git().Stash.Drop(stashEntries[i].Index)
|
||||
self.c.Refresh(types.RefreshOptions{Scope: []types.RefreshableView{types.STASH}})
|
||||
if err != nil {
|
||||
if err := self.c.Git().Stash.Drop(stashEntries[i].Index); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -35,10 +35,13 @@ func (gui *Gui) promptEditor(v *gocui.View, key gocui.Key) bool {
|
|||
v.RenderTextArea()
|
||||
|
||||
suggestionsContext := gui.State.Contexts.Suggestions
|
||||
if suggestionsContext.State.FindSuggestions != nil {
|
||||
// Capture the suggestions function and the input here, on the UI thread; the
|
||||
// main thread rewrites State.FindSuggestions when it (re)creates a prompt
|
||||
// panel, so reading it from the worker below would race that write.
|
||||
if findSuggestions := suggestionsContext.State.FindSuggestions; findSuggestions != nil {
|
||||
input := v.TextArea.GetContent()
|
||||
suggestionsContext.State.AsyncHandler.Do(func() func() {
|
||||
suggestions := suggestionsContext.State.FindSuggestions(input)
|
||||
suggestions := findSuggestions(input)
|
||||
return func() { suggestionsContext.SetSuggestions(suggestions) }
|
||||
})
|
||||
}
|
||||
|
|
|
|||
|
|
@ -49,7 +49,6 @@ import (
|
|||
"github.com/jesseduffield/lazygit/pkg/utils"
|
||||
"github.com/samber/lo"
|
||||
"github.com/sasha-s/go-deadlock"
|
||||
"gopkg.in/ozeidan/fuzzy-patricia.v3/patricia"
|
||||
)
|
||||
|
||||
const StartupPopupVersion = 5
|
||||
|
|
@ -620,9 +619,7 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context {
|
|||
|
||||
// setting this to nil so we don't get stuck based on a popup that was
|
||||
// previously opened
|
||||
gui.Mutexes.PopupMutex.Lock()
|
||||
gui.State.CurrentPopupOpts = nil
|
||||
gui.Mutexes.PopupMutex.Unlock()
|
||||
|
||||
return gui.c.Context().Current()
|
||||
}
|
||||
|
|
@ -641,7 +638,6 @@ func (gui *Gui) resetState(startArgs appTypes.StartArgs) types.Context {
|
|||
FilteredReflogCommits: make([]*models.Commit, 0),
|
||||
ReflogCommits: make([]*models.Commit, 0),
|
||||
BisectInfo: git_commands.NewNullBisectInfo(),
|
||||
FilesTrie: patricia.NewTrie(),
|
||||
Authors: map[string]*models.Author{},
|
||||
MainBranches: git_commands.NewMainBranches(gui.c.Common, gui.os.Cmd),
|
||||
HashPool: &utils.StringPool{},
|
||||
|
|
@ -812,13 +808,25 @@ func NewGui(
|
|||
|
||||
gui.PopupHandler = popup.NewPopupHandler(
|
||||
cmn,
|
||||
// Raising a popup or menu pushes a context and mutates the popup views,
|
||||
// and it can be triggered from a worker goroutine (e.g. a
|
||||
// WithWaitingStatus handler that hits a merge conflict and asks the user
|
||||
// how to proceed). Bounce the creation onto the UI thread so it can't
|
||||
// race the layout/draw code. Doing it here, at the one point where these
|
||||
// producers are injected, keeps every caller oblivious to the threading.
|
||||
func(ctx goContext.Context, opts types.CreatePopupPanelOpts) {
|
||||
gui.helpers.Confirmation.CreatePopupPanel(ctx, opts)
|
||||
gui.onUIThread(func() error {
|
||||
gui.helpers.Confirmation.CreatePopupPanel(ctx, opts)
|
||||
return nil
|
||||
})
|
||||
},
|
||||
func() error { gui.c.Refresh(types.RefreshOptions{}); return nil },
|
||||
func() { gui.State.ContextMgr.Pop() },
|
||||
func() types.Context { return gui.State.ContextMgr.Current() },
|
||||
gui.createMenu,
|
||||
func(opts types.CreateMenuOptions) error {
|
||||
gui.onUIThread(func() error { return gui.createMenu(opts) })
|
||||
return nil
|
||||
},
|
||||
func(message string, f func(gocui.Task) error) { gui.helpers.AppStatus.WithWaitingStatus(message, f) },
|
||||
func(message string, f func(gocui.Task) error) {
|
||||
gui.helpers.AppStatus.WithWaitingStatusBlockingInput(message, f)
|
||||
|
|
|
|||
|
|
@ -58,7 +58,18 @@ func (self *guiCommon) PauseBackgroundRefreshes(pause bool) {
|
|||
self.gui.BackgroundRoutineMgr.PauseBackgroundRefreshes(pause)
|
||||
}
|
||||
|
||||
// assertOnUIThread panics (in debug builds) if called from a worker goroutine.
|
||||
// Use it to guard accessors for state that only the UI thread may touch, so
|
||||
// that a stray worker access fails deterministically -- and points at itself --
|
||||
// rather than surfacing later as a probabilistic data race.
|
||||
func (self *guiCommon) assertOnUIThread(accessor string) {
|
||||
if self.GetConfig().GetDebug() && !self.GocuiGui().IsUIThread() {
|
||||
panic(accessor + " accessed from a worker")
|
||||
}
|
||||
}
|
||||
|
||||
func (self *guiCommon) Context() types.IContextMgr {
|
||||
self.assertOnUIThread("Context()")
|
||||
return self.gui.State.ContextMgr
|
||||
}
|
||||
|
||||
|
|
@ -113,6 +124,7 @@ func (self *guiCommon) Modes() *types.Modes {
|
|||
}
|
||||
|
||||
func (self *guiCommon) Model() *types.Model {
|
||||
self.assertOnUIThread("Model()")
|
||||
return self.gui.State.Model
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -92,7 +92,10 @@ func (self *GuiDriver) Keys() config.KeybindingConfig {
|
|||
}
|
||||
|
||||
func (self *GuiDriver) CurrentContext() types.Context {
|
||||
return self.gui.c.Context().Current()
|
||||
// Read the context manager directly rather than through c.Context(): the
|
||||
// driver runs on the test goroutine, not the UI thread, so it must bypass
|
||||
// the UI-thread assertion that accessor carries.
|
||||
return self.gui.State.ContextMgr.Current()
|
||||
}
|
||||
|
||||
func (self *GuiDriver) ContextForView(viewName string) types.Context {
|
||||
|
|
|
|||
|
|
@ -93,9 +93,18 @@ func (gui *Gui) newPtyTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
|
||||
manager := gui.getManager(view)
|
||||
|
||||
// Size the pty from the view's dimensions here, on the UI thread; the
|
||||
// start func below runs on the task's goroutine, which must not read the
|
||||
// view's live dimensions while the UI thread is laying it out.
|
||||
cols, rows := gui.desiredPtySize(view)
|
||||
|
||||
var p oscommands.Pty
|
||||
start := func() (tasks.Cmd, io.Reader) {
|
||||
cols, rows := gui.desiredPtySize(view)
|
||||
// The pty (and pager) wrap to this width; apply it here, on the
|
||||
// task's goroutine once the previous task has stopped, so it doesn't
|
||||
// race that task's writes (see View.SetContentWidth).
|
||||
view.SetContentWidth(width)
|
||||
|
||||
sp, err := oscommands.StartPty(cmd, cols, rows)
|
||||
if err != nil {
|
||||
gui.c.Log.Error(err)
|
||||
|
|
|
|||
|
|
@ -18,8 +18,17 @@ func (gui *Gui) newCmdTask(view *gocui.View, cmd *exec.Cmd, prefix string) error
|
|||
|
||||
manager := gui.getManager(view)
|
||||
|
||||
// Snapshot the view width here, on the UI thread, so the task goroutine
|
||||
// doesn't read the view's live dimensions while it streams output. It's
|
||||
// applied inside start() below rather than now, because start() runs once
|
||||
// the previous task has stopped -- applying it here would race that task's
|
||||
// still-running writes (see View.SetContentWidth).
|
||||
contentWidth := view.InnerWidth()
|
||||
|
||||
var r io.ReadCloser
|
||||
start := func() (tasks.Cmd, io.Reader) {
|
||||
view.SetContentWidth(contentWidth)
|
||||
|
||||
var err error
|
||||
r, err = cmd.StdoutPipe()
|
||||
if err != nil {
|
||||
|
|
@ -59,8 +68,10 @@ func (gui *Gui) newStringTaskWithoutScroll(view *gocui.View, str string) error {
|
|||
manager := gui.getManager(view)
|
||||
|
||||
f := func(tasks.TaskOpts) error {
|
||||
gui.c.SetViewContent(view, str)
|
||||
return nil
|
||||
return gui.g.OnUIThreadAndWaitBackground(func() error {
|
||||
gui.c.SetViewContent(view, str)
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
if err := manager.NewTask(f, manager.GetTaskKey()); err != nil {
|
||||
|
|
@ -74,9 +85,11 @@ func (gui *Gui) newStringTaskWithScroll(view *gocui.View, str string, originX in
|
|||
manager := gui.getManager(view)
|
||||
|
||||
f := func(tasks.TaskOpts) error {
|
||||
gui.c.SetViewContent(view, str)
|
||||
view.SetOrigin(originX, originY)
|
||||
return nil
|
||||
return gui.g.OnUIThreadAndWaitBackground(func() error {
|
||||
gui.c.SetViewContent(view, str)
|
||||
view.SetOrigin(originX, originY)
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
if err := manager.NewTask(f, manager.GetTaskKey()); err != nil {
|
||||
|
|
@ -90,9 +103,11 @@ func (gui *Gui) newStringTaskWithKey(view *gocui.View, str string, key string) e
|
|||
manager := gui.getManager(view)
|
||||
|
||||
f := func(tasks.TaskOpts) error {
|
||||
gui.c.ResetViewOrigin(view)
|
||||
gui.c.SetViewContent(view, str)
|
||||
return nil
|
||||
return gui.g.OnUIThreadAndWaitBackground(func() error {
|
||||
gui.c.ResetViewOrigin(view)
|
||||
gui.c.SetViewContent(view, str)
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
if err := manager.NewTask(f, key); err != nil {
|
||||
|
|
@ -150,6 +165,9 @@ func (gui *Gui) getManager(view *gocui.View) *tasks.ViewBufferManager {
|
|||
// otherwise make the switch that handler triggers refuse itself.
|
||||
return gui.c.GocuiGui().NewBackgroundTask()
|
||||
},
|
||||
// Rendering is background work too (see above), so the view mutations
|
||||
// it bounces onto the UI thread mustn't count towards being busy.
|
||||
gui.g.OnUIThreadAndWaitBackground,
|
||||
)
|
||||
gui.viewBufferManagerMap[view.Name()] = manager
|
||||
}
|
||||
|
|
|
|||
|
|
@ -40,11 +40,8 @@ func (gui *Gui) handleTestMode() {
|
|||
return gocui.ErrQuit
|
||||
})
|
||||
|
||||
waitUntilIdle()
|
||||
|
||||
time.Sleep(time.Second * 1)
|
||||
|
||||
log.Fatal("gocui should have already exited")
|
||||
// Wait for the event loop to actually exit.
|
||||
<-gui.g.LoopExited()
|
||||
}()
|
||||
|
||||
if os.Getenv(components.WAIT_FOR_DEBUGGER_ENV_VAR) == "" {
|
||||
|
|
|
|||
|
|
@ -11,7 +11,6 @@ import (
|
|||
"github.com/jesseduffield/lazygit/pkg/tasks"
|
||||
"github.com/jesseduffield/lazygit/pkg/utils"
|
||||
"github.com/sasha-s/go-deadlock"
|
||||
"gopkg.in/ozeidan/fuzzy-patricia.v3/patricia"
|
||||
)
|
||||
|
||||
type HelperCommon struct {
|
||||
|
|
@ -348,9 +347,6 @@ type Model struct {
|
|||
|
||||
MainBranches *git_commands.MainBranches
|
||||
|
||||
// for displaying suggestions while typing in a file name
|
||||
FilesTrie *patricia.Trie
|
||||
|
||||
Authors map[string]*models.Author
|
||||
|
||||
HashPool *utils.StringPool
|
||||
|
|
@ -358,7 +354,6 @@ type Model struct {
|
|||
|
||||
type Mutexes struct {
|
||||
SubprocessMutex deadlock.Mutex
|
||||
PopupMutex deadlock.Mutex
|
||||
PtyMutex deadlock.Mutex
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -56,7 +56,7 @@ func TestIntegration(t *testing.T) {
|
|||
CodeCoverageDir: codeCoverageDir,
|
||||
InputDelay: 0,
|
||||
// Allow two attempts at each test to get around flakiness
|
||||
MaxAttempts: 2,
|
||||
MaxAttempts: 1,
|
||||
})
|
||||
|
||||
assert.NoError(t, err)
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ import (
|
|||
"os"
|
||||
"os/exec"
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"time"
|
||||
|
||||
"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
|
||||
|
|
@ -59,9 +60,13 @@ type ViewBufferManager struct {
|
|||
taskIDMutex deadlock.Mutex
|
||||
Log *logrus.Entry
|
||||
newTaskID int
|
||||
readLines chan LinesToRead
|
||||
taskKey string
|
||||
onNewKey func()
|
||||
// The channel by which the currently-running task is told to read more
|
||||
// lines (e.g. as the user scrolls). Held in an atomic because it's swapped
|
||||
// out as tasks come and go while ReadLines/ReadToEnd read it from the UI
|
||||
// thread; nil when no task is running.
|
||||
readLines atomic.Pointer[chan LinesToRead]
|
||||
taskKey string
|
||||
onNewKey func()
|
||||
|
||||
// beforeStart is the function that is called before starting a new task
|
||||
beforeStart func()
|
||||
|
|
@ -74,11 +79,18 @@ type ViewBufferManager struct {
|
|||
// whereas the tasks in this file are about rendering content to a view.
|
||||
newGocuiTask func() gocui.Task
|
||||
|
||||
// Runs f on the UI thread and blocks until it has completed. All mutations
|
||||
// of the view happen through this, so that the view is only ever touched on
|
||||
// the UI thread (where it is also laid out and drawn), never on the task's
|
||||
// own goroutine.
|
||||
onUIThread func(f func() error) error
|
||||
|
||||
// if the user flicks through a heap of items, with each one
|
||||
// spawning a process to render something to the main view,
|
||||
// it can slow things down quite a bit. In these situations we
|
||||
// want to throttle the spawning of processes.
|
||||
throttle bool
|
||||
// want to throttle the spawning of processes. Atomic because it's set
|
||||
// from one task's stop goroutine and read when the next task starts.
|
||||
throttle atomic.Bool
|
||||
}
|
||||
|
||||
type LinesToRead struct {
|
||||
|
|
@ -110,6 +122,7 @@ func NewViewBufferManager(
|
|||
onEndOfInput func(),
|
||||
onNewKey func(),
|
||||
newGocuiTask func() gocui.Task,
|
||||
onUIThread func(f func() error) error,
|
||||
) *ViewBufferManager {
|
||||
return &ViewBufferManager{
|
||||
Log: log,
|
||||
|
|
@ -117,9 +130,9 @@ func NewViewBufferManager(
|
|||
beforeStart: beforeStart,
|
||||
refreshView: refreshView,
|
||||
onEndOfInput: onEndOfInput,
|
||||
readLines: nil,
|
||||
onNewKey: onNewKey,
|
||||
newGocuiTask: newGocuiTask,
|
||||
onUIThread: onUIThread,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -128,17 +141,19 @@ func NewViewBufferManager(
|
|||
// (e.g. as the user scrolls down, back up, and down again) don't re-read lines
|
||||
// that have already been read: the task only ever reads the shortfall.
|
||||
func (self *ViewBufferManager) ReadLines(totalLines int) {
|
||||
if self.readLines != nil {
|
||||
if ch := self.readLines.Load(); ch != nil {
|
||||
readLines := *ch
|
||||
go utils.Safe(func() {
|
||||
self.readLines <- LinesToRead{Total: totalLines, InitialRefreshAfter: -1}
|
||||
readLines <- LinesToRead{Total: totalLines, InitialRefreshAfter: -1}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func (self *ViewBufferManager) ReadToEnd(then func()) {
|
||||
if self.readLines != nil {
|
||||
if ch := self.readLines.Load(); ch != nil {
|
||||
readLines := *ch
|
||||
go utils.Safe(func() {
|
||||
self.readLines <- LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: then}
|
||||
readLines <- LinesToRead{Total: -1, InitialRefreshAfter: -1, Then: then}
|
||||
})
|
||||
} else if then != nil {
|
||||
then()
|
||||
|
|
@ -163,7 +178,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
onFirstPageShown()
|
||||
}
|
||||
|
||||
if self.throttle {
|
||||
if self.throttle.Load() {
|
||||
self.Log.Info("throttling task")
|
||||
time.Sleep(THROTTLE_TIME)
|
||||
}
|
||||
|
|
@ -186,13 +201,13 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
case <-done:
|
||||
// The command finished and did not have to be preemptively stopped before the next command.
|
||||
// No need to throttle.
|
||||
self.throttle = false
|
||||
self.throttle.Store(false)
|
||||
case <-opts.Stop:
|
||||
// we use the time it took to start the program as a way of checking if things
|
||||
// are running slow at the moment. This is admittedly a crude estimate, but
|
||||
// the point is that we only want to throttle when things are running slow
|
||||
// and the user is flicking through a bunch of items.
|
||||
self.throttle = time.Since(startTime) < THROTTLE_TIME && timeToStart > COMMAND_START_THRESHOLD
|
||||
self.throttle.Store(time.Since(startTime) < THROTTLE_TIME && timeToStart > COMMAND_START_THRESHOLD)
|
||||
|
||||
// Kill the still-running command. The only reason to do this is to save CPU usage
|
||||
// when flicking through several very long diffs when diff.algorithm = histogram is
|
||||
|
|
@ -212,7 +227,8 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
|
||||
loadingMutex := deadlock.Mutex{}
|
||||
|
||||
self.readLines = make(chan LinesToRead, 1024)
|
||||
readLines := make(chan LinesToRead, 1024)
|
||||
self.readLines.Store(&readLines)
|
||||
|
||||
scanner := bufio.NewScanner(r)
|
||||
scanner.Split(utils.ScanLinesAndTruncateWhenLongerThanBuffer(bufio.MaxScanTokenSize))
|
||||
|
|
@ -304,7 +320,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
select {
|
||||
case <-opts.Stop:
|
||||
break outer
|
||||
case linesToRead := <-self.readLines:
|
||||
case linesToRead := <-readLines:
|
||||
callThen := func() {
|
||||
if linesToRead.Then != nil {
|
||||
linesToRead.Then()
|
||||
|
|
@ -337,8 +353,14 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
|
||||
if !ok {
|
||||
// if we're here then there's nothing left to scan from the source
|
||||
// so we're at the EOF and can flush the stale content
|
||||
self.onEndOfInput()
|
||||
// so we're at the EOF and can flush the stale content.
|
||||
// onEndOfInput reads the view's dimensions (to decide
|
||||
// whether to scroll) and sets the origin, both of which
|
||||
// are UI-thread-only, so run it there.
|
||||
_ = self.onUIThread(func() error {
|
||||
self.onEndOfInput()
|
||||
return nil
|
||||
})
|
||||
callThen()
|
||||
break outer
|
||||
}
|
||||
|
|
@ -359,7 +381,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
}
|
||||
}
|
||||
|
||||
self.readLines = nil
|
||||
self.readLines.Store(nil)
|
||||
|
||||
refreshViewIfStale()
|
||||
|
||||
|
|
@ -383,7 +405,7 @@ func (self *ViewBufferManager) NewCmdTask(start func() (Cmd, io.Reader), prefix
|
|||
close(lineWrittenChan)
|
||||
})
|
||||
|
||||
self.readLines <- linesToRead
|
||||
readLines <- linesToRead
|
||||
|
||||
<-done
|
||||
|
||||
|
|
@ -460,21 +482,31 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
|
|||
self.taskIDMutex.Lock()
|
||||
|
||||
// Bail out before touching shared view state if a newer task has
|
||||
// already been queued: if we ran onNewKey here we'd reset the view
|
||||
// for a task that's about to exit, potentially wiping output the
|
||||
// winning task has already written.
|
||||
// already been queued: if we reset the view here we'd do it for a task
|
||||
// that's about to exit, potentially wiping output the winning task has
|
||||
// already written.
|
||||
if taskID < self.newTaskID {
|
||||
self.taskIDMutex.Unlock()
|
||||
return
|
||||
}
|
||||
|
||||
if self.GetTaskKey() != key && self.onNewKey != nil {
|
||||
self.onNewKey()
|
||||
}
|
||||
resetOrigin := self.GetTaskKey() != key && self.onNewKey != nil
|
||||
self.taskKey = key
|
||||
|
||||
self.taskIDMutex.Unlock()
|
||||
|
||||
if resetOrigin {
|
||||
// onNewKey resets the view's scroll origin, which is view state the
|
||||
// UI thread reads while laying out and drawing, so do it there. This
|
||||
// must happen after releasing taskIDMutex: it blocks until the UI
|
||||
// thread runs it, and a NewTask call on the UI thread takes
|
||||
// taskIDMutex, so holding it here would deadlock.
|
||||
_ = self.onUIThread(func() error {
|
||||
self.onNewKey()
|
||||
return nil
|
||||
})
|
||||
}
|
||||
|
||||
self.waitingMutex.Lock()
|
||||
|
||||
// Re-check staleness after acquiring waitingMutex: a newer task
|
||||
|
|
@ -491,7 +523,7 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
|
|||
self.stopCurrentTask()
|
||||
}
|
||||
|
||||
self.readLines = nil
|
||||
self.readLines.Store(nil)
|
||||
|
||||
stop := make(chan struct{})
|
||||
notifyStopped := make(chan struct{})
|
||||
|
|
|
|||
|
|
@ -39,6 +39,8 @@ func TestNewCmdTaskInstantStop(t *testing.T) {
|
|||
onEndOfInput,
|
||||
onNewKey,
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
|
|
@ -104,6 +106,8 @@ func TestNewCmdTask(t *testing.T) {
|
|||
onEndOfInput,
|
||||
onNewKey,
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
|
|
@ -237,6 +241,8 @@ func TestNewCmdTaskRefresh(t *testing.T) {
|
|||
func() {},
|
||||
func() {},
|
||||
newTask,
|
||||
// no UI thread in the test; run the view mutations inline
|
||||
func(f func() error) error { return f() },
|
||||
)
|
||||
|
||||
stop := make(chan struct{})
|
||||
|
|
|
|||
Loading…
Reference in a new issue