maildir: fix data race in maildir worker

Fix a data race due dirInfo pointer being read in the main goroutine
by NewMessageStore and written in the anonymous goroutine launched in
Worker.getDirectoryInfo.

To address the issue raised in https://todo.sr.ht/~rjarry/aerc/16, we
use readdir(3) once, parse and cache its results, this replaces
go-maildir library Dir.Flags based  stat(3) and filepath.Glob
causing the issue when N (emails) is large.

Signed-off-by: wagner riffel <w@104d.net>
This commit is contained in:
wagner riffel 2022-03-06 10:06:34 +01:00 committed by Robin Jarry
parent c1636f8d75
commit 55ae3d2cab
1 changed files with 82 additions and 25 deletions

View File

@ -9,6 +9,8 @@ import (
"net/url" "net/url"
"os" "os"
"path/filepath" "path/filepath"
"sort"
"strings"
"github.com/emersion/go-maildir" "github.com/emersion/go-maildir"
"github.com/fsnotify/fsnotify" "github.com/fsnotify/fsnotify"
@ -120,6 +122,36 @@ func (w *Worker) err(msg types.WorkerMessage, err error) {
}, nil) }, nil)
} }
func splitMaildirFile(name string) (uniq string, flags []maildir.Flag, err error) {
i := strings.LastIndexByte(name, ':')
if i < 0 {
return "", nil, &maildir.MailfileError{Name: name}
}
info := name[i+1:]
uniq = name[:i]
if len(info) < 2 {
return "", nil, &maildir.FlagError{Info: info, Experimental: false}
}
if info[1] != ',' || info[0] != '2' {
return "", nil, &maildir.FlagError{Info: info, Experimental: false}
}
if info[0] == '1' {
return "", nil, &maildir.FlagError{Info: info, Experimental: true}
}
flags = []maildir.Flag(info[2:])
sort.Slice(flags, func(i, j int) bool { return info[i] < info[j] })
return uniq, flags, nil
}
func dirFiles(name string) ([]string, error) {
dir, err := os.Open(filepath.Join(name, "cur"))
if err != nil {
return nil, err
}
defer dir.Close()
return dir.Readdirnames(-1)
}
func (w *Worker) getDirectoryInfo(name string) *models.DirectoryInfo { func (w *Worker) getDirectoryInfo(name string) *models.DirectoryInfo {
dirInfo := &models.DirectoryInfo{ dirInfo := &models.DirectoryInfo{
Name: name, Name: name,
@ -136,6 +168,21 @@ func (w *Worker) getDirectoryInfo(name string) *models.DirectoryInfo {
} }
dir := w.c.Dir(name) dir := w.c.Dir(name)
var keyFlags map[string][]maildir.Flag
files, err := dirFiles(string(dir))
if err == nil {
keyFlags = make(map[string][]maildir.Flag, len(files))
for _, v := range files {
key, flags, err := splitMaildirFile(v)
if err != nil {
w.worker.Logger.Printf("%q: error parsing flags (%q): %v", v, key, err)
continue
}
keyFlags[key] = flags
}
} else {
w.worker.Logger.Printf("disabled flags cache: %q: %v", dir, err)
}
uids, err := w.c.UIDs(dir) uids, err := w.c.UIDs(dir)
if err != nil { if err != nil {
@ -144,38 +191,48 @@ func (w *Worker) getDirectoryInfo(name string) *models.DirectoryInfo {
} }
dirInfo.Exists = len(uids) dirInfo.Exists = len(uids)
go func() {
info := dirInfo
for _, uid := range uids { for _, uid := range uids {
message, err := w.c.Message(dir, uid) message, err := w.c.Message(dir, uid)
if err != nil { if err != nil {
w.worker.Logger.Printf("could not get message: %v", err) w.worker.Logger.Printf("could not get message: %v", err)
continue continue
} }
flags, err := message.Flags() var flags []maildir.Flag
if keyFlags != nil {
ok := false
flags, ok = keyFlags[message.key]
if !ok {
w.worker.Logger.Printf("message (key=%q uid=%d) not found in map cache", message.key, message.uid)
flags, err = message.Flags()
if err != nil { if err != nil {
w.worker.Logger.Printf("could not get flags: %v", err) w.worker.Logger.Printf("could not get flags: %v", err)
continue continue
} }
}
} else {
flags, err = message.Flags()
if err != nil {
w.worker.Logger.Printf("could not get flags: %v", err)
continue
}
}
seen := false seen := false
for _, flag := range flags { for _, flag := range flags {
if flag == maildir.FlagSeen { if flag == maildir.FlagSeen {
seen = true seen = true
break
} }
} }
if !seen { if !seen {
info.Unseen++ dirInfo.Unseen++
} }
if w.c.IsRecent(uid) { if w.c.IsRecent(uid) {
info.Recent++ dirInfo.Recent++
} }
} }
info.Unseen += info.Recent dirInfo.Unseen += dirInfo.Recent
info.Exists += info.Recent dirInfo.Exists += dirInfo.Recent
info.AccurateCounts = true dirInfo.AccurateCounts = true
}()
return dirInfo return dirInfo
} }