notmuch/maildir: remove double emit of the dirinfo v1 PROPOSED

Reto Brunner: 1
 notmuch/maildir: remove double emit of the dirinfo

 2 files changed, 3 insertions(+), 7 deletions(-)

[PATCH] notmuch/maildir: remove double emit of the dirinfo Export this patch

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 <w@104d.net>
---
Considering that I can't reproduce the bug, let's just get rid of it until someone
complains and then investigate.

@Wagner: Happy with me including your name?

wagner riffel <w@104d.net>

2 years ago

Sure thing, no problem.
The patch LGTM.

 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
 }
 
-- 
2.31.1