Hang when showing diffs

[stefanhaller]

We have a race condition with command tasks, introduced with 60887ed (as part of #4429, Focus the main view).

It doesn't happen very often, but I can reproduce it fairly consistently on my machine by going into a longer interactive rebase (press e like 20 commits down from head), and then moving one of the pick todos up or down with auto-repeat). The main view then gets stuck and no longer updates when I move from one commit to another.

I think what happens is that a new command task is spawned and initializes its readlines channel here, and then, before it gets to read from the channel, it is set to nil again for the next task here, so that when it gets to reading the initial lines here, it tries to read from a nil channel, which blocks forever.

We then get a deadlock because the new task is blocking on acquiring the waitingMutex here, which is still held by the previous task. This part is a bit tricky to understand, because we just saw that the previous task blocks on reading from the readlines channel, which only happens after it released the waitingMutex again. So I think this can only be explained when we consider three tasks. My current theory is that task 1 gets started, locks and unlocks the waitingMutex, and initialized the readlines channel; then task 2 comes along, sets the readlines channel back to nil, locks the waitingMutex successfully, calls stopCurrentTask, which blocks on reading from the notifyStopped channel (of task 1); it blocks there forever because task 1 never gets to closing that channel. Finally, task 3 comes along and blocks on the waitingMutex.

Phew.

It looks like this can be fixed by moving the setting of the readlines channel to nil to after the stopCurrentTask call, i.e.:

diff --git a/pkg/tasks/tasks.go b/pkg/tasks/tasks.go
index e20e58045..bc56c5110 100644
--- a/pkg/tasks/tasks.go
+++ b/pkg/tasks/tasks.go
@@ -373,8 +373,6 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 	go utils.Safe(func() {
 		defer completeGocuiTask()
 
-		self.readLines = nil
-
 		self.taskIDMutex.Lock()
 		self.newTaskID++
 		taskID := self.newTaskID
@@ -400,6 +398,8 @@ func (self *ViewBufferManager) NewTask(f func(TaskOpts) error, key string) error
 			self.stopCurrentTask()
 		}
 
+		self.readLines = nil
+
 		stop := make(chan struct{})
 		notifyStopped := make(chan struct{})
 

At first I wasn't really convinced that this is a good enough fix, but after thinking through all the above again, I guess it may be.