lib/msgstore: protect with a mutex

MessageStore has a lot of exported fields that can be read from the outside.
Each read must be protected, because a call from Update could happen at any
time.
This commit is contained in:
Simon Ser 2019-04-28 13:26:38 +00:00 committed by Drew DeVault
parent f1698a337e
commit a275f65848
2 changed files with 40 additions and 3 deletions

View file

@ -2,6 +2,7 @@ package lib
import ( import (
"io" "io"
"sync"
"time" "time"
"github.com/emersion/go-imap" "github.com/emersion/go-imap"
@ -9,7 +10,10 @@ import (
"git.sr.ht/~sircmpwn/aerc2/worker/types" "git.sr.ht/~sircmpwn/aerc2/worker/types"
) )
// Accesses to fields must be guarded by MessageStore.Lock/Unlock
type MessageStore struct { type MessageStore struct {
sync.Mutex
Deleted map[uint32]interface{} Deleted map[uint32]interface{}
DirInfo types.DirectoryInfo DirInfo types.DirectoryInfo
Messages map[uint32]*types.MessageInfo Messages map[uint32]*types.MessageInfo
@ -45,6 +49,9 @@ func NewMessageStore(worker *types.Worker,
func (store *MessageStore) FetchHeaders(uids []uint32, func (store *MessageStore) FetchHeaders(uids []uint32,
cb func(*types.MessageInfo)) { cb func(*types.MessageInfo)) {
store.Lock()
defer store.Unlock()
// TODO: this could be optimized by pre-allocating toFetch and trimming it // TODO: this could be optimized by pre-allocating toFetch and trimming it
// at the end. In practice we expect to get most messages back in one frame. // at the end. In practice we expect to get most messages back in one frame.
var toFetch imap.SeqSet var toFetch imap.SeqSet
@ -67,6 +74,9 @@ func (store *MessageStore) FetchHeaders(uids []uint32,
} }
func (store *MessageStore) FetchFull(uids []uint32, cb func(io.Reader)) { func (store *MessageStore) FetchFull(uids []uint32, cb func(io.Reader)) {
store.Lock()
defer store.Unlock()
// TODO: this could be optimized by pre-allocating toFetch and trimming it // TODO: this could be optimized by pre-allocating toFetch and trimming it
// at the end. In practice we expect to get most messages back in one frame. // at the end. In practice we expect to get most messages back in one frame.
var toFetch imap.SeqSet var toFetch imap.SeqSet
@ -103,8 +113,7 @@ func (store *MessageStore) FetchBodyPart(
}) })
} }
func (store *MessageStore) merge( func merge(to *types.MessageInfo, from *types.MessageInfo) {
to *types.MessageInfo, from *types.MessageInfo) {
if from.BodyStructure != nil { if from.BodyStructure != nil {
to.BodyStructure = from.BodyStructure to.BodyStructure = from.BodyStructure
@ -125,6 +134,8 @@ func (store *MessageStore) merge(
} }
func (store *MessageStore) Update(msg types.WorkerMessage) { func (store *MessageStore) Update(msg types.WorkerMessage) {
store.Lock()
update := false update := false
switch msg := msg.(type) { switch msg := msg.(type) {
case *types.DirectoryInfo: case *types.DirectoryInfo:
@ -144,7 +155,7 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
update = true update = true
case *types.MessageInfo: case *types.MessageInfo:
if existing, ok := store.Messages[msg.Uid]; ok && existing != nil { if existing, ok := store.Messages[msg.Uid]; ok && existing != nil {
store.merge(existing, msg) merge(existing, msg)
} else { } else {
store.Messages[msg.Uid] = msg store.Messages[msg.Uid] = msg
} }
@ -186,6 +197,9 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
store.Uids = uids store.Uids = uids
update = true update = true
} }
store.Unlock()
if update { if update {
store.update() store.update()
} }
@ -202,11 +216,16 @@ func (store *MessageStore) update() {
} }
func (store *MessageStore) Delete(uids []uint32) { func (store *MessageStore) Delete(uids []uint32) {
store.Lock()
var set imap.SeqSet var set imap.SeqSet
for _, uid := range uids { for _, uid := range uids {
set.AddNum(uid) set.AddNum(uid)
store.Deleted[uid] = nil store.Deleted[uid] = nil
} }
store.Unlock()
store.worker.PostAction(&types.DeleteMessages{Uids: set}, nil) store.worker.PostAction(&types.DeleteMessages{Uids: set}, nil)
store.update() store.update()
} }

View file

@ -53,6 +53,8 @@ func (ml *MessageList) Draw(ctx *ui.Context) {
return return
} }
store.Lock()
var ( var (
needsHeaders []uint32 needsHeaders []uint32
row int = 0 row int = 0
@ -92,6 +94,8 @@ func (ml *MessageList) Draw(ctx *ui.Context) {
tcell.StyleDefault, "%s", msg) tcell.StyleDefault, "%s", msg)
} }
store.Unlock()
if len(needsHeaders) != 0 { if len(needsHeaders) != 0 {
store.FetchHeaders(needsHeaders, nil) store.FetchHeaders(needsHeaders, nil)
ml.spinner.Start() ml.spinner.Start()
@ -109,11 +113,13 @@ func (ml *MessageList) storeUpdate(store *lib.MessageStore) {
return return
} }
store.Lock()
if len(store.Uids) > 0 { if len(store.Uids) > 0 {
for ml.selected >= len(store.Uids) { for ml.selected >= len(store.Uids) {
ml.Prev() ml.Prev()
} }
} }
store.Unlock()
ml.Invalidate() ml.Invalidate()
} }
@ -139,16 +145,25 @@ func (ml *MessageList) Store() *lib.MessageStore {
func (ml *MessageList) Empty() bool { func (ml *MessageList) Empty() bool {
store := ml.Store() store := ml.Store()
store.Lock()
defer store.Unlock()
return store == nil || len(store.Uids) == 0 return store == nil || len(store.Uids) == 0
} }
func (ml *MessageList) Selected() *types.MessageInfo { func (ml *MessageList) Selected() *types.MessageInfo {
store := ml.Store() store := ml.Store()
store.Lock()
defer store.Unlock()
return store.Messages[store.Uids[len(store.Uids)-ml.selected-1]] return store.Messages[store.Uids[len(store.Uids)-ml.selected-1]]
} }
func (ml *MessageList) Select(index int) { func (ml *MessageList) Select(index int) {
store := ml.Store() store := ml.Store()
store.Lock()
defer store.Unlock()
ml.selected = index ml.selected = index
for ; ml.selected < 0; ml.selected = len(store.Uids) + ml.selected { for ; ml.selected < 0; ml.selected = len(store.Uids) + ml.selected {
} }
@ -166,6 +181,9 @@ func (ml *MessageList) Select(index int) {
func (ml *MessageList) nextPrev(delta int) { func (ml *MessageList) nextPrev(delta int) {
store := ml.Store() store := ml.Store()
store.Lock()
defer store.Unlock()
if store == nil || len(store.Uids) == 0 { if store == nil || len(store.Uids) == 0 {
return return
} }