diff options
author | Tim Culverhouse <tim@timculverhouse.com> | 2023-08-30 13:08:24 -0500 |
---|---|---|
committer | Robin Jarry <robin@jarry.cc> | 2023-08-30 22:11:03 +0200 |
commit | a8ac5d3c4ca9a55a4cdc320f8163255a72edd917 (patch) | |
tree | a9c7868c5b0ae7e0b50b516a515793d6a0fbfbe4 /widgets/dirlist.go | |
parent | f8747089b3ef26ffd0b8d673419654335417be8e (diff) | |
download | aerc-a8ac5d3c4ca9a55a4cdc320f8163255a72edd917.tar.gz |
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>
Diffstat (limited to 'widgets/dirlist.go')
-rw-r--r-- | widgets/dirlist.go | 56 |
1 files changed, 30 insertions, 26 deletions
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 |