summaryrefslogtreecommitdiff
path: root/worker/imap/seqmap_test.go
diff options
context:
space:
mode:
authorRobin Jarry <robin@jarry.cc>2022-06-20 21:49:05 +0200
committerRobin Jarry <robin@jarry.cc>2022-06-24 21:08:12 +0200
commit420f236d31da76df9982331c596b776ab3d0dd76 (patch)
treedf1c0ee7612400aefd08f0ce260bbb421d50b491 /worker/imap/seqmap_test.go
parent389c0f82a7f5dc250500fcff6570308cc3bcbd88 (diff)
downloadaerc-420f236d31da76df9982331c596b776ab3d0dd76.zip
imap: fix data race on seqMap array
There are concurrent threads that are accessing and modifying IMAPWorker.seqMap (the mapping of sequence numbers to message UIDs). This can lead to crashes when trying to add and remove a message ID. panic: runtime error: index out of range [391] with length 390 goroutine 1834 [running]: git.sr.ht/~rjarry/aerc/logging.PanicHandler() logging/panic-logger.go:47 +0x6de panic({0xa41760, 0xc0019b3290}) /usr/lib/golang/src/runtime/panic.go:838 +0x207 git.sr.ht/~rjarry/aerc/worker/imap.(*IMAPWorker).handleFetchMessages.func1() worker/imap/fetch.go:214 +0x185 created by git.sr.ht/~rjarry/aerc/worker/imap.(*IMAPWorker).handleFetchMessages worker/imap/fetch.go:209 +0x12b Use a map which makes more sense than a simple array for random access operations. Also, it allows better typing for the key values. Protect the map with a mutex. Add internal API to access the map. Add basic unit tests to ensure that concurrent access works. Fixes: https://todo.sr.ht/~rjarry/aerc/49 Signed-off-by: Robin Jarry <robin@jarry.cc> Acked-by: Moritz Poldrack <moritz@poldrack.dev>
Diffstat (limited to 'worker/imap/seqmap_test.go')
-rw-r--r--worker/imap/seqmap_test.go78
1 files changed, 78 insertions, 0 deletions
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)
+}