dirlist: fix capturing of dirlist vars when selecting directory

The directory selecting logic allows for a delay in issuing the command
to the backend. This delay is part of a goroutine which is supposed to
capture variables intended to be used in a callback to set the selected
directory. The callback does not capture the variable properly,
specifically the context and the "dirlist.selecting" value. This results
in contexts not being cancelled properly and incorrect selection logic.

The flow of the issue only occurs when the delay is sufficiently low:
1. The user scrolls rapidly through the directory list. Each time
   passing over a directory, dirlist.selecting is set to that directory
   and a context is created for this selection.
2. If the delay is low enough that the context was not cancelled before
   the Action was posted, the worker could actually open this directory.
   The captured context is actually referencing to the _current_ context
   of the dirlist, and so even if this OpenDirectory Action makes it to
   the worker, which might properly check to see if the context is
   cancelled, it will be referring always to the current context and not
   be cancelled.
3. When posting back the Done result, the callback is processed.
   dirlist.selecting and the context are no longer referring to the
   values used when this Action was made. The backend thinks it has
   opened Directory A, but the callback sets the dirlist.selected to
   Directory B
4. The account widget grabs the selected msgstore (Directory B, even
   though the backend thinks its A). Sort messages are called, and all
   sorts of things are sent to the backend which now is out of sync.
5. Eventually this all comes back into sync once the correct directory
   has churned it's way through the worker and back to the account.

Move the callback into the dirlist.Update method to ensure proper
capture of all variables involved. Only reference the values set in the
message instead of those referring to the dirlist. This ensures that the
worker and UI are always in agreement for which directory is selected.

Signed-off-by: Tim Culverhouse <tim@timculverhouse.com>
Acked-by: Robin Jarry <robin@jarry.cc>
Reviewed-by: Koni Marti <koni.marti@gmail.com>

2 files changed, 31 insertions, 26 deletions

diff --git a/widgets/account.go b/widgets/account.go
index d4a74ce8..47bc9924 100644
--- a/widgets/account.go
+++ b/widgets/account.go
@@ -277,6 +277,7 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 			log.Infof("[%s] disconnected.", acct.acct.Name)
 			acct.SetStatus(state.SetConnected(false))
 		case *types.OpenDirectory:
+			acct.dirlist.Update(msg)
 			if store, ok := acct.dirlist.SelectedMsgStore(); ok {
 				// If we've opened this dir before, we can re-render it from
 				// memory while we wait for the update and the UI feels
diff --git a/widgets/dirlist.go b/widgets/dirlist.go
index b58a9a5c..e9cec458 100644
--- a/widgets/dirlist.go
+++ b/widgets/dirlist.go
@@ -108,6 +108,28 @@ func (dirlist *DirectoryList) Update(msg types.WorkerMessage) {
 	switch msg := msg.(type) {
 	case *types.Done:
 		switch msg := msg.InResponseTo().(type) {
+		case *types.OpenDirectory:
+			dirlist.selected = msg.Directory
+			dirlist.filterDirsByFoldersConfig()
+			hasSelected := false
+			for _, d := range dirlist.dirs {
+				if d == dirlist.selected {
+					hasSelected = true
+					break
+				}
+			}
+			if !hasSelected && dirlist.selected != "" {
+				dirlist.dirs = append(dirlist.dirs, dirlist.selected)
+			}
+			if dirlist.acctConf.EnableFoldersSort {
+				sort.Strings(dirlist.dirs)
+			}
+			dirlist.sortDirsByFoldersSortConfig()
+			store, ok := dirlist.SelectedMsgStore()
+			if !ok {
+				return
+			}
+			store.SetContext(msg.Context)
 		case *types.ListDirectories:
 			dirlist.filterDirsByFoldersConfig()
 			dirlist.sortDirsByFoldersSortConfig()
@@ -162,39 +184,21 @@ func (dirlist *DirectoryList) Select(name string) {
 		select {
 		case <-time.After(delay):
 			dirlist.worker.PostAction(&types.OpenDirectory{
-				Context:   dirlist.ctx,
+				Context:   ctx,
 				Directory: name,
 			},
 				func(msg types.WorkerMessage) {
-					switch msg.(type) {
+					switch msg := msg.(type) {
 					case *types.Error:
 						dirlist.selecting = ""
-					case *types.Done:
-						dirlist.selected = dirlist.selecting
-						dirlist.filterDirsByFoldersConfig()
-						hasSelected := false
-						for _, d := range dirlist.dirs {
-							if d == dirlist.selected {
-								hasSelected = true
-								break
-							}
-						}
-						if !hasSelected && dirlist.selected != "" {
-							dirlist.dirs = append(dirlist.dirs, dirlist.selected)
-						}
-						if dirlist.acctConf.EnableFoldersSort {
-							sort.Strings(dirlist.dirs)
-						}
-						dirlist.sortDirsByFoldersSortConfig()
-						store, ok := dirlist.SelectedMsgStore()
-						if !ok {
-							return
-						}
-						store.SetContext(dirlist.ctx)
+						log.Errorf("(%s) couldn't open directory %s: %v",
+							dirlist.acctConf.Name,
+							name,
+							msg.Error)
+					case *types.Cancelled:
+						log.Debugf("OpenDirectory cancelled")
 					}
-					dirlist.Invalidate()
 				})
-			dirlist.Invalidate()
 		case <-ctx.Done():
 			log.Tracef("dirlist: skip %s", name)
 			return