From 1687e558d3fae4d5622677c031bb9365a8c3e261 Mon Sep 17 00:00:00 2001 From: Reto Brunner Date: Mon, 26 Apr 2021 21:36:00 +0200 Subject: [PATCH] notmuch/maildir: remove double emit of the dirinfo There was some bug which could be worked around by double emitting an event. However that proofed to be brittle: We send the first message here from the worker goroutine: https://git.sr.ht/~sircmpwn/aerc/tree/a5553438/item/worker/maildir/worker.g= o#L306 Then Tick() is waked in the main goroutine and calls ProcessMessage: https://git.sr.ht/~sircmpwn/aerc/tree/a5553438/item/widgets/account.go#L100 ProcessMessage in the main goroutine reads types.Message state with msg.getId() and msg.InResponseTo(): https://git.sr.ht/~sircmpwn/aerc/tree/a5553438/item/worker/types/worker.go#= L74-76 Meanwhile in the worker goroutine we call PostMessage for a second time with a pointer that points to the *same* previous message that ProcessMessage is reading: https://git.sr.ht/~sircmpwn/aerc/tree/a5553438/item/worker/maildir/worker.g= o#L306 The second PostMessage call makes writes to message while ProcessMessage in the main goroutine is possibly reading: https://git.sr.ht/~sircmpwn/aerc/tree/a5553438/item/worker/types/worker.go#= L59 This led to a data race in the event loop Reported-By: Wagner Riffel --- worker/maildir/worker.go | 2 -- worker/notmuch/worker.go | 8 +++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go index 4a7ae51..87ebc97 100644 --- a/worker/maildir/worker.go +++ b/worker/maildir/worker.go @@ -299,12 +299,10 @@ func (w *Worker) handleOpenDirectory(msg *types.OpenDirectory) error { return fmt.Errorf("could not clean directory: %v", err) } - // TODO: why does this need to be sent twice?? info := &types.DirectoryInfo{ Info: w.getDirectoryInfo(msg.Directory), } w.worker.PostMessage(info, nil) - w.worker.PostMessage(info, nil) return nil } diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go index 6281744..637bb4d 100644 --- a/worker/notmuch/worker.go +++ b/worker/notmuch/worker.go @@ -251,8 +251,6 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error { return err } info.Message = types.RespondTo(msg) - //TODO: why does this need to be sent twice?? - w.w.PostMessage(info, nil) w.w.PostMessage(info, nil) w.done(msg) return nil @@ -507,9 +505,9 @@ func (w *worker) loadExcludeTags( return nil } excludedTags := strings.Split(raw, ",") - for idx, tag := range excludedTags { - excludedTags[idx] = strings.Trim(tag, " ") - } + for idx, tag := range excludedTags { + excludedTags[idx] = strings.Trim(tag, " ") + } return excludedTags }