Many Drawable implementations have their own Invalidate and OnInvalidate
functions, with an unexported onInvalidate field. However OnInvalidate and
Invalidate are usually not called in the same goroutine. This results in a race
on this field, e.g.:
Read at 0x00c000094748 by goroutine 7:
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList.func1()
/home/simon/src/aerc2/widgets/dirlist.go:85 +0x56
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1()
/home/simon/src/aerc2/widgets/spinner.go:93 +0x1bb
Previous write at 0x00c000094748 by main goroutine:
[failed to restore the stack]
Goroutine 7 (running) created at:
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start()
/home/simon/src/aerc2/widgets/spinner.go:46 +0x8f
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList()
/home/simon/src/aerc2/widgets/dirlist.go:37 +0x286
git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView()
/home/simon/src/aerc2/widgets/account.go:50 +0x5ca
git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc()
/home/simon/src/aerc2/widgets/aerc.go:60 +0x800
main.main()
/home/simon/src/aerc2/aerc.go:65 +0x33e
To fix this, introduce a new type, Invalidatable, which protects the field.
Unfortunately the Drawable must be passed to the callback function in
Invalidate, so we still need to re-implement this in each Invalidatable user.
Worker.Process* functions were called in different goroutines than
Worker.Post*. Protect the map with a mutex. Also make the map unexported to
prevent external unprotected accesses.
Worker.Process* functions used to delete items from the map. However they
didn't delete the element they retrieved: callbacks[msg.InResponseTo()] was
read while callbacks[msg] was deleted. I'm not sure I understand why. I tried
to delete the element that was accessed - but this broke everything (UI froze
at "Connecting..."). I don't believe any elements were actually removed from
the map, so the new code just doesn't remove anything.
It's accessed by the goroutine which increments it and the goroutine that draws
the widget at the same time. Use atomic instead.
Write at 0x00c00000ebc0 by goroutine 7:
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start.func1()
/home/simon/src/aerc2/widgets/spinner.go:50 +0x169
Previous read at 0x00c00000ebc0 by main goroutine:
[failed to restore the stack]
Goroutine 7 (running) created at:
git.sr.ht/~sircmpwn/aerc2/widgets.(*Spinner).Start()
/home/simon/src/aerc2/widgets/spinner.go:44 +0x8b
git.sr.ht/~sircmpwn/aerc2/widgets.NewDirectoryList()
/home/simon/src/aerc2/widgets/dirlist.go:37 +0x286
git.sr.ht/~sircmpwn/aerc2/widgets.NewAccountView()
/home/simon/src/aerc2/widgets/account.go:50 +0x5ca
git.sr.ht/~sircmpwn/aerc2/widgets.NewAerc()
/home/simon/src/aerc2/widgets/aerc.go:60 +0x800
main.main()
/home/simon/src/aerc2/aerc.go:65 +0x33e
When changing to an empty directory, ml.selected is 0, and the length
of ml.store.Uids is 0. The loop condition is always true so we have
an infinite loop causing 100% CPU usage and prevents us to change to
other directories.
Signed-off-by: Tom Lebreux <tomlebreux@cock.li>
Simon Ser
Elias Naur
Tom Lebreux
Drew DeVault
Gokberk Yaltirakli
Julian P Samaroo