diff --git a/worker/imap/fetch.go b/worker/imap/fetch.go index e8a8251..7d7b72f 100644 --- a/worker/imap/fetch.go +++ b/worker/imap/fetch.go @@ -211,7 +211,7 @@ func (imapw *IMAPWorker) handleFetchMessages( var reterr error for _msg := range messages { - imapw.seqMap[_msg.SeqNum-1] = _msg.Uid + imapw.seqMap.Put(_msg.SeqNum, _msg.Uid) err := procFunc(_msg) if err != nil { if reterr == nil { diff --git a/worker/imap/flags.go b/worker/imap/flags.go index 22c23dd..2bded2a 100644 --- a/worker/imap/flags.go +++ b/worker/imap/flags.go @@ -2,6 +2,7 @@ package imap import ( "fmt" + "github.com/emersion/go-imap" "git.sr.ht/~rjarry/aerc/logging" @@ -26,9 +27,11 @@ func (imapw *IMAPWorker) handleDeleteMessages(msg *types.DeleteMessages) { defer logging.PanicHandler() for seqNum := range ch { - i := seqNum - 1 - deleted = append(deleted, imapw.seqMap[i]) - imapw.seqMap = append(imapw.seqMap[:i], imapw.seqMap[i+1:]...) + if uid, found := imapw.seqMap.Pop(seqNum); !found { + imapw.worker.Logger.Printf("handleDeleteMessages unknown seqnum: %v", seqNum) + } else { + deleted = append(deleted, uid) + } } done <- nil }() diff --git a/worker/imap/open.go b/worker/imap/open.go index 65060fe..a0607d0 100644 --- a/worker/imap/open.go +++ b/worker/imap/open.go @@ -61,9 +61,7 @@ func (imapw *IMAPWorker) handleFetchDirectoryContents( }, nil) } else { imapw.worker.Logger.Printf("Found %d UIDs", len(uids)) - if len(imapw.seqMap) < len(uids) { - imapw.seqMap = make([]uint32, len(uids)) - } + imapw.seqMap.Clear() imapw.worker.PostMessage(&types.DirectoryContents{ Message: types.RespondTo(msg), Uids: uids, @@ -113,7 +111,7 @@ func (imapw *IMAPWorker) handleDirectoryThreaded( aercThreads, count := convertThreads(threads, nil) sort.Sort(types.ByUID(aercThreads)) imapw.worker.Logger.Printf("Found %d threaded messages", count) - imapw.seqMap = make([]uint32, count) + imapw.seqMap.Clear() imapw.worker.PostMessage(&types.DirectoryThreaded{ Message: types.RespondTo(msg), Threads: aercThreads, diff --git a/worker/imap/seqmap.go b/worker/imap/seqmap.go new file mode 100644 index 0000000..2752cc8 --- /dev/null +++ b/worker/imap/seqmap.go @@ -0,0 +1,48 @@ +package imap + +import "sync" + +type SeqMap struct { + lock sync.Mutex + // map of IMAP sequence numbers to message UIDs + m map[uint32]uint32 +} + +func (s *SeqMap) Size() int { + s.lock.Lock() + size := len(s.m) + s.lock.Unlock() + return size +} + +func (s *SeqMap) Get(seqnum uint32) (uint32, bool) { + s.lock.Lock() + uid, found := s.m[seqnum] + s.lock.Unlock() + return uid, found +} + +func (s *SeqMap) Put(seqnum, uid uint32) { + s.lock.Lock() + if s.m == nil { + s.m = make(map[uint32]uint32) + } + s.m[seqnum] = uid + s.lock.Unlock() +} + +func (s *SeqMap) Pop(seqnum uint32) (uint32, bool) { + s.lock.Lock() + uid, found := s.m[seqnum] + if found { + delete(s.m, seqnum) + } + s.lock.Unlock() + return uid, found +} + +func (s *SeqMap) Clear() { + s.lock.Lock() + s.m = make(map[uint32]uint32) + s.lock.Unlock() +} diff --git a/worker/imap/seqmap_test.go b/worker/imap/seqmap_test.go new file mode 100644 index 0000000..0ee0738 --- /dev/null +++ b/worker/imap/seqmap_test.go @@ -0,0 +1,78 @@ +package imap + +import ( + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" +) + +func TestSeqMap(t *testing.T) { + var seqmap SeqMap + var uid uint32 + var found bool + assert := assert.New(t) + + assert.Equal(seqmap.Size(), 0) + + _, found = seqmap.Get(42) + assert.Equal(found, false) + + _, found = seqmap.Pop(0) + assert.Equal(found, false) + + seqmap.Put(1, 1337) + seqmap.Put(2, 42) + seqmap.Put(3, 1107) + assert.Equal(seqmap.Size(), 3) + + _, found = seqmap.Pop(0) + assert.Equal(found, false) + + uid, found = seqmap.Get(1) + assert.Equal(uid, uint32(1337)) + assert.Equal(found, true) + + uid, found = seqmap.Pop(1) + assert.Equal(uid, uint32(1337)) + assert.Equal(found, true) + assert.Equal(seqmap.Size(), 2) + + _, found = seqmap.Pop(1) + assert.Equal(found, false) + assert.Equal(seqmap.Size(), 2) + + seqmap.Put(1, 7331) + assert.Equal(seqmap.Size(), 3) + + seqmap.Clear() + assert.Equal(seqmap.Size(), 0) + + var wg sync.WaitGroup + wg.Add(1) + go func() { + defer wg.Done() + time.Sleep(20 * time.Millisecond) + seqmap.Put(42, 1337) + time.Sleep(20 * time.Millisecond) + seqmap.Put(43, 1107) + }() + wg.Add(1) + go func() { + defer wg.Done() + for _, found := seqmap.Pop(43); !found; _, found = seqmap.Pop(43) { + time.Sleep(1 * time.Millisecond) + } + }() + wg.Add(1) + go func() { + defer wg.Done() + for _, found := seqmap.Pop(42); !found; _, found = seqmap.Pop(42) { + time.Sleep(1 * time.Millisecond) + } + }() + wg.Wait() + + assert.Equal(seqmap.Size(), 0) +} diff --git a/worker/imap/worker.go b/worker/imap/worker.go index e890bb8..7debd88 100644 --- a/worker/imap/worker.go +++ b/worker/imap/worker.go @@ -61,8 +61,7 @@ type IMAPWorker struct { selected *imap.MailboxStatus updates chan client.Update worker *types.Worker - // Map of sequence numbers to UIDs, index 0 is seq number 1 - seqMap []uint32 + seqMap SeqMap idler *idler observer *observer @@ -212,12 +211,6 @@ func (w *IMAPWorker) handleMessage(msg types.WorkerMessage) error { func (w *IMAPWorker) handleImapUpdate(update client.Update) { w.worker.Logger.Printf("(= %T", update) - checkBounds := func(idx, size int) bool { - if idx < 0 || idx >= size { - return false - } - return true - } switch update := update.(type) { case *client.MailboxUpdate: status := update.Mailbox @@ -238,11 +231,12 @@ func (w *IMAPWorker) handleImapUpdate(update client.Update) { case *client.MessageUpdate: msg := update.Message if msg.Uid == 0 { - if ok := checkBounds(int(msg.SeqNum)-1, len(w.seqMap)); !ok { - w.worker.Logger.Println("MessageUpdate error: index out of range") + if uid, found := w.seqMap.Get(msg.SeqNum); !found { + w.worker.Logger.Printf("MessageUpdate unknown seqnum: %v", msg.SeqNum) return + } else { + msg.Uid = uid } - msg.Uid = w.seqMap[msg.SeqNum-1] } w.worker.PostMessage(&types.MessageInfo{ Info: &models.MessageInfo{ @@ -254,16 +248,13 @@ func (w *IMAPWorker) handleImapUpdate(update client.Update) { }, }, nil) case *client.ExpungeUpdate: - i := update.SeqNum - 1 - if ok := checkBounds(int(i), len(w.seqMap)); !ok { - w.worker.Logger.Println("ExpungeUpdate error: index out of range") - return + if uid, found := w.seqMap.Pop(update.SeqNum); !found { + w.worker.Logger.Printf("ExpungeUpdate unknown seqnum: %v", update.SeqNum) + } else { + w.worker.PostMessage(&types.MessagesDeleted{ + Uids: []uint32{uid}, + }, nil) } - uid := w.seqMap[i] - w.seqMap = append(w.seqMap[:i], w.seqMap[i+1:]...) - w.worker.PostMessage(&types.MessagesDeleted{ - Uids: []uint32{uid}, - }, nil) } }