Mitigate Apple Mail re-sync (both bodies and meta info)

This commit is contained in:
Michal Horejsek
2020-12-22 12:06:19 +01:00
parent 5117672388
commit 516ca018d3
24 changed files with 239 additions and 225 deletions

View File

@ -37,7 +37,7 @@ type panicHandler interface {
type imapBackend struct {
panicHandler panicHandler
bridge bridger
updates chan goIMAPBackend.Update
updates *imapUpdates
eventListener listener.Listener
users map[string]*imapUser
@ -46,9 +46,6 @@ type imapBackend struct {
imapCache map[string]map[string]string
imapCachePath string
imapCacheLock *sync.RWMutex
updatesBlocking map[string]bool
updatesBlockingLocker sync.Locker
}
// NewIMAPBackend returns struct implementing go-imap/backend interface.
@ -75,7 +72,7 @@ func newIMAPBackend(
return &imapBackend{
panicHandler: panicHandler,
bridge: bridge,
updates: make(chan goIMAPBackend.Update),
updates: newIMAPUpdates(),
eventListener: eventListener,
users: map[string]*imapUser{},
@ -83,9 +80,6 @@ func newIMAPBackend(
imapCachePath: cache.GetIMAPCachePath(),
imapCacheLock: &sync.RWMutex{},
updatesBlocking: map[string]bool{},
updatesBlockingLocker: &sync.Mutex{},
}
}
@ -172,7 +166,7 @@ func (ib *imapBackend) Login(_ *imap.ConnInfo, username, password string) (goIMA
// so that it doesn't make bridge slow for users who are only using bridge for SMTP
// (otherwise the store will be locked for 1 sec per email during synchronization).
if store := imapUser.user.GetStore(); store != nil {
store.SetChangeNotifier(ib)
store.SetChangeNotifier(ib.updates)
}
return imapUser, nil
@ -183,7 +177,7 @@ func (ib *imapBackend) Updates() <-chan goIMAPBackend.Update {
// Called from go-imap in goroutines - we need to handle panics for each function.
defer ib.panicHandler.HandlePanic()
return ib.updates
return ib.updates.ch
}
func (ib *imapBackend) CreateMessageLimit() *uint32 {

View File

@ -188,8 +188,8 @@ func (im *imapMailbox) Expunge() error {
// the desired mailbox.
im.user.waitForAppend()
im.user.backend.setUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
defer im.user.backend.unsetUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
im.user.backend.updates.block(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
defer im.user.backend.updates.unblock(im.user.currentAddressLowercase, im.name, operationDeleteMessage)
return im.storeMailbox.RemoveDeleted(nil)
}

View File

@ -263,6 +263,7 @@ func (im *imapMailbox) getMessage(storeMessage storeMessageProvider, items []ima
// Size attribute on the server counts encrypted data. The value is cleared
// on our part and we need to compute "real" size of decrypted data.
if m.Size <= 0 {
im.log.WithField("msgID", storeMessage.ID()).Trace("Size unknown - downloading body")
if _, _, err = im.getBodyStructure(storeMessage); err != nil {
return
}
@ -308,14 +309,14 @@ func (im *imapMailbox) getBodyStructure(storeMessage storeMessageProvider) (
if bodyReader, structure = cache.LoadMail(id); bodyReader.Len() == 0 || structure == nil {
var body []byte
structure, body, err = im.buildMessage(m)
m.Size = int64(len(body))
if err := storeMessage.SetSize(m.Size); err != nil {
im.log.WithError(err).
WithField("newSize", m.Size).
WithField("msgID", m.ID).
Warn("Cannot update size while building")
}
if err == nil && structure != nil && len(body) > 0 {
m.Size = int64(len(body))
if err := storeMessage.SetSize(m.Size); err != nil {
im.log.WithError(err).
WithField("newSize", m.Size).
WithField("msgID", m.ID).
Warn("Cannot update size while building")
}
if err := storeMessage.SetContentTypeAndHeader(m.MIMEType, m.Header); err != nil {
im.log.WithError(err).
WithField("msgID", m.ID).

View File

@ -46,8 +46,8 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat
// Called from go-imap in goroutines - we need to handle panics for each function.
defer im.panicHandler.HandlePanic()
im.user.backend.setUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
defer im.user.backend.unsetUpdatesBeBlocking(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
im.user.backend.updates.block(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
defer im.user.backend.updates.unblock(im.user.currentAddressLowercase, im.name, operationUpdateMessage)
messageIDs, err := im.apiIDsFromSeqSet(uid, seqSet)
if err != nil || len(messageIDs) == 0 {
@ -455,8 +455,8 @@ func (im *imapMailbox) ListMessages(isUID bool, seqSet *imap.SeqSet, items []ima
// EXPUNGE cannot be sent during listing and can come only from
// the event loop, so we prevent any server side update to avoid
// the problem.
im.user.pauseEventLoop()
defer im.user.unpauseEventLoop()
im.user.backend.updates.forbidExpunge(im.storeMailbox.LabelID())
defer im.user.backend.updates.allowExpunge(im.storeMailbox.LabelID())
}
var markAsReadIDs []string

View File

@ -43,16 +43,15 @@ import (
)
type imapServer struct {
panicHandler panicHandler
server *imapserver.Server
eventListener listener.Listener
debugClient bool
debugServer bool
on bool
}
// NewIMAPServer constructs a new IMAP server configured with the given options.
func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, imapBackend *imapBackend, eventListener listener.Listener) *imapServer { //nolint[golint]
func NewIMAPServer(panicHandler panicHandler, debugClient, debugServer bool, port int, tls *tls.Config, imapBackend *imapBackend, eventListener listener.Listener) *imapServer { //nolint[golint]
s := imapserver.New(imapBackend)
s.Addr = fmt.Sprintf("%v:%v", bridge.Host, port)
s.TLSConfig = tls
@ -98,6 +97,7 @@ func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, ima
)
return &imapServer{
panicHandler: panicHandler,
server: s,
eventListener: eventListener,
debugClient: debugClient,
@ -114,14 +114,6 @@ func (s *imapServer) ListenAndServe() {
}
func (s *imapServer) listenAndServe() {
if s.on {
return
}
s.on = true
defer func() {
s.on = false
}()
log.Info("IMAP server listening at ", s.server.Addr)
l, err := net.Listen("tcp", s.server.Addr)
if err != nil {
@ -135,9 +127,18 @@ func (s *imapServer) listenAndServe() {
server: s,
})
if err != nil {
s.eventListener.Emit(events.ErrorEvent, "IMAP failed: "+err.Error())
log.Error("IMAP failed: ", err)
return
failed := true
if netErr, ok := err.(*net.OpError); ok {
originalErr := netErr.Unwrap()
if originalErr != nil && originalErr.Error() == "use of closed network connection" {
failed = false
}
}
if failed {
s.eventListener.Emit(events.ErrorEvent, "IMAP failed: "+err.Error())
log.Error("IMAP failed: ", err)
return
}
}
defer s.server.Close() //nolint[errcheck]
@ -146,6 +147,7 @@ func (s *imapServer) listenAndServe() {
// Stops the server.
func (s *imapServer) Close() {
log.Info("Closing IMAP server")
if err := s.server.Close(); err != nil {
log.WithError(err).Error("Failed to close the connection")
}
@ -157,16 +159,30 @@ func (s *imapServer) monitorInternetConnection() {
off := make(chan string)
s.eventListener.Add(events.InternetOffEvent, off)
go func() {
for range on {
s.listenAndServe()
}
}()
go func() {
for range off {
isOn := true
for {
select {
case <-on:
if isOn {
continue
}
isOn = true
go func() {
defer s.panicHandler.HandlePanic()
s.listenAndServe()
}()
case <-off:
if !isOn {
continue
}
isOn = false
s.Close()
}
}()
// Give it some time to serve or close server before changing it again.
// E.g., if we get quickly off-on signal, starting or closing could
// fail because server is still running or not yet, respectively.
time.Sleep(10 * time.Second)
}
}
func (s *imapServer) monitorDisconnectedUsers() {

View File

@ -42,8 +42,6 @@ type storeUserProvider interface {
attachedPublicKeyName string,
parentID string) (*pmapi.Message, []*pmapi.Attachment, error)
PauseEventLoop(bool)
SetChangeNotifier(store.ChangeNotifier)
}

View File

@ -19,6 +19,7 @@ package imap
import (
"strings"
"sync"
"time"
"github.com/ProtonMail/proton-bridge/internal/store"
@ -36,32 +37,82 @@ const (
operationDeleteMessage operation = "expunge"
)
func (ib *imapBackend) setUpdatesBeBlocking(address, mailboxName string, op operation) {
ib.changeUpdatesBlocking(address, mailboxName, op, true)
type imapUpdates struct {
lock sync.Locker
blocking map[string]bool
delayedExpunges map[string][]chan struct{}
ch chan goIMAPBackend.Update
}
func (ib *imapBackend) unsetUpdatesBeBlocking(address, mailboxName string, op operation) {
ib.changeUpdatesBlocking(address, mailboxName, op, false)
}
func (ib *imapBackend) changeUpdatesBlocking(address, mailboxName string, op operation, block bool) {
ib.updatesBlockingLocker.Lock()
defer ib.updatesBlockingLocker.Unlock()
key := strings.ToLower(address + "_" + mailboxName + "_" + string(op))
if block {
ib.updatesBlocking[key] = true
} else {
delete(ib.updatesBlocking, key)
func newIMAPUpdates() *imapUpdates {
return &imapUpdates{
lock: &sync.Mutex{},
blocking: map[string]bool{},
delayedExpunges: map[string][]chan struct{}{},
ch: make(chan goIMAPBackend.Update),
}
}
func (ib *imapBackend) isBlocking(address, mailboxName string, op operation) bool {
key := strings.ToLower(address + "_" + mailboxName + "_" + string(op))
return ib.updatesBlocking[key]
func (iu *imapUpdates) block(address, mailboxName string, op operation) {
iu.lock.Lock()
defer iu.lock.Unlock()
iu.blocking[getBlockingKey(address, mailboxName, op)] = true
}
func (ib *imapBackend) Notice(address, notice string) {
func (iu *imapUpdates) unblock(address, mailboxName string, op operation) {
iu.lock.Lock()
defer iu.lock.Unlock()
delete(iu.blocking, getBlockingKey(address, mailboxName, op))
}
func (iu *imapUpdates) isBlocking(address, mailboxName string, op operation) bool {
iu.lock.Lock()
defer iu.lock.Unlock()
return iu.blocking[getBlockingKey(address, mailboxName, op)]
}
func getBlockingKey(address, mailboxName string, op operation) string {
return strings.ToLower(address + "_" + mailboxName + "_" + string(op))
}
func (iu *imapUpdates) forbidExpunge(mailboxID string) {
iu.lock.Lock()
defer iu.lock.Unlock()
iu.delayedExpunges[mailboxID] = []chan struct{}{}
}
func (iu *imapUpdates) allowExpunge(mailboxID string) {
iu.lock.Lock()
defer iu.lock.Unlock()
for _, ch := range iu.delayedExpunges[mailboxID] {
close(ch)
}
delete(iu.delayedExpunges, mailboxID)
}
func (iu *imapUpdates) CanDelete(mailboxID string) (bool, func()) {
iu.lock.Lock()
defer iu.lock.Unlock()
if iu.delayedExpunges[mailboxID] == nil {
return true, nil
}
ch := make(chan struct{})
iu.delayedExpunges[mailboxID] = append(iu.delayedExpunges[mailboxID], ch)
return false, func() {
log.WithField("mailbox", mailboxID).Debug("Expunge operations paused")
<-ch
log.WithField("mailbox", mailboxID).Debug("Expunge operations unpaused")
}
}
func (iu *imapUpdates) Notice(address, notice string) {
update := new(goIMAPBackend.StatusUpdate)
update.Update = goIMAPBackend.NewUpdate(address, "")
update.StatusResp = &imap.StatusResp{
@ -69,10 +120,10 @@ func (ib *imapBackend) Notice(address, notice string) {
Code: imap.CodeAlert,
Info: notice,
}
ib.sendIMAPUpdate(update, false)
iu.sendIMAPUpdate(update, false)
}
func (ib *imapBackend) UpdateMessage(
func (iu *imapUpdates) UpdateMessage(
address, mailboxName string,
uid, sequenceNumber uint32,
msg *pmapi.Message, hasDeletedFlag bool,
@ -93,10 +144,10 @@ func (ib *imapBackend) UpdateMessage(
update.Message.Flags = append(update.Message.Flags, imap.DeletedFlag)
}
update.Message.Uid = uid
ib.sendIMAPUpdate(update, ib.isBlocking(address, mailboxName, operationUpdateMessage))
iu.sendIMAPUpdate(update, iu.isBlocking(address, mailboxName, operationUpdateMessage))
}
func (ib *imapBackend) DeleteMessage(address, mailboxName string, sequenceNumber uint32) {
func (iu *imapUpdates) DeleteMessage(address, mailboxName string, sequenceNumber uint32) {
log.WithFields(logrus.Fields{
"address": address,
"mailbox": mailboxName,
@ -105,10 +156,10 @@ func (ib *imapBackend) DeleteMessage(address, mailboxName string, sequenceNumber
update := new(goIMAPBackend.ExpungeUpdate)
update.Update = goIMAPBackend.NewUpdate(address, mailboxName)
update.SeqNum = sequenceNumber
ib.sendIMAPUpdate(update, ib.isBlocking(address, mailboxName, operationDeleteMessage))
iu.sendIMAPUpdate(update, iu.isBlocking(address, mailboxName, operationDeleteMessage))
}
func (ib *imapBackend) MailboxCreated(address, mailboxName string) {
func (iu *imapUpdates) MailboxCreated(address, mailboxName string) {
log.WithFields(logrus.Fields{
"address": address,
"mailbox": mailboxName,
@ -120,10 +171,10 @@ func (ib *imapBackend) MailboxCreated(address, mailboxName string) {
Delimiter: store.PathDelimiter,
Name: mailboxName,
}
ib.sendIMAPUpdate(update, false)
iu.sendIMAPUpdate(update, false)
}
func (ib *imapBackend) MailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint32) {
func (iu *imapUpdates) MailboxStatus(address, mailboxName string, total, unread, unreadSeqNum uint32) {
log.WithFields(logrus.Fields{
"address": address,
"mailbox": mailboxName,
@ -137,11 +188,11 @@ func (ib *imapBackend) MailboxStatus(address, mailboxName string, total, unread,
update.MailboxStatus.Messages = total
update.MailboxStatus.Unseen = unread
update.MailboxStatus.UnseenSeqNum = unreadSeqNum
ib.sendIMAPUpdate(update, false)
iu.sendIMAPUpdate(update, false)
}
func (ib *imapBackend) sendIMAPUpdate(update goIMAPBackend.Update, block bool) {
if ib.updates == nil {
func (iu *imapUpdates) sendIMAPUpdate(update goIMAPBackend.Update, block bool) {
if iu.ch == nil {
log.Trace("IMAP IDLE unavailable")
return
}
@ -152,7 +203,7 @@ func (ib *imapBackend) sendIMAPUpdate(update goIMAPBackend.Update, block bool) {
case <-time.After(1 * time.Second):
log.Warn("IMAP update could not be sent (timeout)")
return
case ib.updates <- update:
case iu.ch <- update:
}
}()

View File

@ -42,9 +42,6 @@ type imapUser struct {
currentAddressLowercase string
appendInProcess sync.WaitGroup
eventLoopPausingCounter int
eventLoopPausingLocker sync.Locker
}
// newIMAPUser returns struct implementing go-imap/user interface.
@ -76,8 +73,6 @@ func newIMAPUser(
storeAddress: storeAddress,
currentAddressLowercase: strings.ToLower(address),
eventLoopPausingLocker: &sync.Mutex{},
}, err
}
@ -86,31 +81,6 @@ func (iu *imapUser) client() pmapi.Client {
return iu.user.GetTemporaryPMAPIClient()
}
// pauseEventLoop pauses event loop and increases the number of mailboxes which
// is performing action forbidding event loop to run (such as FETCH which needs
// UIDs to be stable and thus EXPUNGE cannot be done during the request).
func (iu *imapUser) pauseEventLoop() {
iu.eventLoopPausingLocker.Lock()
defer iu.eventLoopPausingLocker.Unlock()
iu.eventLoopPausingCounter++
iu.storeUser.PauseEventLoop(true)
}
// unpauseEventLoop unpauses event loop but only if no other request is not
// performing action forbidding event loop to run (see pauseEventLoop).
func (iu *imapUser) unpauseEventLoop() {
iu.eventLoopPausingLocker.Lock()
defer iu.eventLoopPausingLocker.Unlock()
if iu.eventLoopPausingCounter > 0 {
iu.eventLoopPausingCounter--
}
if iu.eventLoopPausingCounter == 0 {
iu.storeUser.PauseEventLoop(false)
}
}
func (iu *imapUser) isSubscribed(labelID string) bool {
subscriptionExceptions := iu.backend.getCacheList(iu.storeUser.UserID(), SubscriptionException)
exceptions := strings.Split(subscriptionExceptions, ";")