better locking of merge panel state

4 files changed, 84 insertions, 36 deletions

diff --git a/pkg/gui/context_config.go b/pkg/gui/context_config.go
index 2a152e517..f567f5a5c 100644
--- a/pkg/gui/context_config.go
+++ b/pkg/gui/context_config.go
@@ -167,7 +167,7 @@ func (gui *Gui) contextTree() ContextTree {
 			Key:      MAIN_PATCH_BUILDING_CONTEXT_KEY,
 		},
 		Merging: &BasicContext{
-			OnFocus:         OnFocusWrapper(gui.renderConflictsWithFocus),
+			OnFocus:         OnFocusWrapper(func() error { return gui.renderConflictsWithLock(true) }),
 			Kind:            MAIN_CONTEXT,
 			ViewName:        "main",
 			Key:             MAIN_MERGING_CONTEXT_KEY,
diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go
index d0b4d736c..fa8cfa79c 100644
--- a/pkg/gui/files_panel.go
+++ b/pkg/gui/files_panel.go
@@ -56,18 +56,16 @@ func (gui *Gui) filesRenderToMain() error {
 	}
 
 	if node.File != nil && node.File.HasInlineMergeConflicts {
-		hasConflicts, err := gui.setMergeState(node.File.Name)
+		ok, err := gui.setConflictsAndRenderWithLock(node.GetPath(), false)
 		if err != nil {
 			return err
 		}
-
-		// if we don't have conflicts we'll fall through and show the diff
-		if hasConflicts {
-			return gui.renderConflicts(false)
+		if ok {
+			return nil
 		}
 	}
 
-	gui.resetMergeState()
+	gui.resetMergeStateWithLock()
 
 	cmdObj := gui.Git.WorkingTree.WorktreeFileDiffCmdObj(node, false, !node.GetHasUnstagedChanges() && node.GetHasStagedChanges(), gui.State.IgnoreWhitespaceInDiffView)
 
@@ -143,28 +141,6 @@ func (gui *Gui) refreshFilesAndSubmodules() error {
 	return nil
 }
 
-func (gui *Gui) refreshMergeState() error {
-	gui.State.Panels.Merging.Lock()
-	defer gui.State.Panels.Merging.Unlock()
-
-	if gui.currentContext().GetKey() != MAIN_MERGING_CONTEXT_KEY {
-		return nil
-	}
-
-	hasConflicts, err := gui.setMergeState(gui.State.Panels.Merging.GetPath())
-	if err != nil {
-		return gui.surfaceError(err)
-	}
-	if hasConflicts {
-		_ = gui.renderConflicts(true)
-	} else {
-		_ = gui.escapeMerge()
-		return nil
-	}
-
-	return nil
-}
-
 // specific functions
 
 func (gui *Gui) stagedFiles() []*models.File {
@@ -959,7 +935,7 @@ func (gui *Gui) switchToMerge() error {
 	gui.takeOverMergeConflictScrolling()
 
 	if gui.State.Panels.Merging.GetPath() != file.Name {
-		hasConflicts, err := gui.setMergeState(file.Name)
+		hasConflicts, err := gui.setMergeStateWithLock(file.Name)
 		if err != nil {
 			return err
 		}
diff --git a/pkg/gui/merge_panel.go b/pkg/gui/merge_panel.go
index 7700f193c..dbd5b3be7 100644
--- a/pkg/gui/merge_panel.go
+++ b/pkg/gui/merge_panel.go
@@ -138,6 +138,13 @@ func (gui *Gui) renderConflicts(hasFocus bool) error {
 		// TODO: find a way to not have to do this OnUIThread thing. Why doesn't it work
 		// without it given that we're calling the 'no scroll' variant below?
 		gui.OnUIThread(func() error {
+			gui.State.Panels.Merging.Lock()
+			defer gui.State.Panels.Merging.Unlock()
+
+			if !state.Active() {
+				return nil
+			}
+
 			gui.centerYPos(gui.Views.Main, state.GetConflictMiddle())
 			return nil
 		})
@@ -156,6 +163,12 @@ func (gui *Gui) renderConflictsWithFocus() error {
 	return gui.renderConflicts(true)
 }
 
+func (gui *Gui) renderConflictsWithLock(hasFocus bool) error {
+	return gui.withMergeConflictLock(func() error {
+		return gui.renderConflicts(hasFocus)
+	})
+}
+
 func (gui *Gui) centerYPos(view *gocui.View, y int) {
 	ox, _ := view.Origin()
 	_, height := view.Size()
@@ -205,15 +218,27 @@ func (gui *Gui) setMergeState(path string) (bool, error) {
 	return !gui.State.Panels.Merging.NoConflicts(), nil
 }
 
-func (gui *Gui) escapeMerge() error {
+func (gui *Gui) setMergeStateWithLock(path string) (bool, error) {
+	gui.State.Panels.Merging.Lock()
+	defer gui.State.Panels.Merging.Unlock()
+
+	return gui.setMergeState(path)
+}
+
+func (gui *Gui) resetMergeStateWithLock() {
+	gui.State.Panels.Merging.Lock()
+	defer gui.State.Panels.Merging.Unlock()
+
 	gui.resetMergeState()
+}
 
-	// it's possible this method won't be called from the merging view so we need to
-	// ensure we only 'return' focus if we already have it
+func (gui *Gui) escapeMerge() error {
+	gui.resetMergeState()
 
-	if gui.currentContext().GetKey() == MAIN_MERGING_CONTEXT_KEY {
+	// doing this in separate UI thread so that we're not still holding the lock by the time refresh the file
+	gui.OnUIThread(func() error {
 		return gui.pushContext(gui.State.Contexts.Files)
-	}
+	})
 	return nil
 }
 
@@ -236,3 +261,44 @@ func (gui *Gui) withMergeConflictLock(f func() error) error {
 func (gui *Gui) takeOverMergeConflictScrolling() {
 	gui.State.Panels.Merging.UserVerticalScrolling = false
 }
+
+func (gui *Gui) setConflictsAndRender(path string, hasFocus bool) (bool, error) {
+	hasConflicts, err := gui.setMergeState(path)
+	if err != nil {
+		return false, err
+	}
+
+	// if we don't have conflicts we'll fall through and show the diff
+	if hasConflicts {
+		return true, gui.renderConflicts(hasFocus)
+	}
+
+	return false, nil
+}
+
+func (gui *Gui) setConflictsAndRenderWithLock(path string, hasFocus bool) (bool, error) {
+	gui.State.Panels.Merging.Lock()
+	defer gui.State.Panels.Merging.Unlock()
+
+	return gui.setConflictsAndRender(path, hasFocus)
+}
+
+func (gui *Gui) refreshMergeState() error {
+	gui.State.Panels.Merging.Lock()
+	defer gui.State.Panels.Merging.Unlock()
+
+	if gui.currentContext().GetKey() != MAIN_MERGING_CONTEXT_KEY {
+		return nil
+	}
+
+	hasConflicts, err := gui.setConflictsAndRender(gui.State.Panels.Merging.GetPath(), true)
+	if err != nil {
+		return gui.surfaceError(err)
+	}
+
+	if !hasConflicts {
+		return gui.escapeMerge()
+	}
+
+	return nil
+}
diff --git a/pkg/gui/mergeconflicts/state.go b/pkg/gui/mergeconflicts/state.go
index 0889aaf41..d84f05545 100644
--- a/pkg/gui/mergeconflicts/state.go
+++ b/pkg/gui/mergeconflicts/state.go
@@ -155,7 +155,13 @@ func (s *State) Active() bool {
 }
 
 func (s *State) GetConflictMiddle() int {
-	return s.currentConflict().target
+	currentConflict := s.currentConflict()
+
+	if currentConflict == nil {
+		return 0
+	}
+
+	return currentConflict.target
 }
 
 func (s *State) ContentAfterConflictResolve(selection Selection) (bool, string, error) {