Skip to content

Commit

Permalink
fix: Fix state connector deadlock
Browse files Browse the repository at this point in the history
Remove `refresh` function as it is no longer required since we removed
the operation queue. Additionally this function was causing a deadlock
on error since the update it was waiting on could not be executed as the
current operation has acquired the database lock.
  • Loading branch information
LBeernaertProton committed Oct 27, 2022
1 parent 5cc3d47 commit 287db37
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 61 deletions.
62 changes: 6 additions & 56 deletions internal/backend/state_connector_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func (sc *stateConnectorImpl) CreateMailbox(ctx context.Context, name []string)
return mbox, nil
}

func (sc *stateConnectorImpl) UpdateMailbox(ctx context.Context, mboxID imap.MailboxID, oldName, newName []string) error {
func (sc *stateConnectorImpl) UpdateMailbox(ctx context.Context, mboxID imap.MailboxID, newName []string) error {
ctx = sc.newContextWithMetadata(ctx)

return sc.connector.UpdateMailboxName(ctx, mboxID, newName)
Expand Down Expand Up @@ -82,11 +82,7 @@ func (sc *stateConnectorImpl) AddMessagesToMailbox(
) error {
ctx = sc.newContextWithMetadata(ctx)

if err := sc.connector.AddMessagesToMailbox(ctx, messageIDs, mboxID); err != nil {
return sc.refresh(ctx, messageIDs, mboxID)
}

return nil
return sc.connector.AddMessagesToMailbox(ctx, messageIDs, mboxID)
}

func (sc *stateConnectorImpl) RemoveMessagesFromMailbox(
Expand All @@ -96,11 +92,7 @@ func (sc *stateConnectorImpl) RemoveMessagesFromMailbox(
) error {
ctx = sc.newContextWithMetadata(ctx)

if err := sc.connector.RemoveMessagesFromMailbox(ctx, messageIDs, mboxID); err != nil {
return sc.refresh(ctx, messageIDs, mboxID)
}

return nil
return sc.connector.RemoveMessagesFromMailbox(ctx, messageIDs, mboxID)
}

func (sc *stateConnectorImpl) MoveMessagesFromMailbox(
Expand All @@ -111,31 +103,19 @@ func (sc *stateConnectorImpl) MoveMessagesFromMailbox(
) error {
ctx = sc.newContextWithMetadata(ctx)

if err := sc.connector.MoveMessages(ctx, messageIDs, mboxFromID, mboxToID); err != nil {
return sc.refresh(ctx, messageIDs, mboxFromID)
}

return nil
return sc.connector.MoveMessages(ctx, messageIDs, mboxFromID, mboxToID)
}

func (sc *stateConnectorImpl) SetMessagesSeen(ctx context.Context, messageIDs []imap.MessageID, seen bool) error {
ctx = sc.newContextWithMetadata(ctx)

if err := sc.connector.MarkMessagesSeen(ctx, messageIDs, seen); err != nil {
return sc.refresh(ctx, messageIDs)
}

return nil
return sc.connector.MarkMessagesSeen(ctx, messageIDs, seen)
}

func (sc *stateConnectorImpl) SetMessagesFlagged(ctx context.Context, messageIDs []imap.MessageID, flagged bool) error {
ctx = sc.newContextWithMetadata(ctx)

if err := sc.connector.MarkMessagesFlagged(ctx, messageIDs, flagged); err != nil {
return sc.refresh(ctx, messageIDs)
}

return nil
return sc.connector.MarkMessagesFlagged(ctx, messageIDs, flagged)
}

func (sc *stateConnectorImpl) SetUIDValidity(uidValidity imap.UID) error {
Expand Down Expand Up @@ -165,33 +145,3 @@ func (sc *stateConnectorImpl) newContextWithMetadata(ctx context.Context) contex

return ctx
}

func (sc *stateConnectorImpl) refresh(ctx context.Context, messageIDs []imap.MessageID, mboxIDs ...imap.MailboxID) error {
for _, messageID := range messageIDs {
message, mboxIDs, err := sc.connector.GetMessage(ctx, messageID)
if err != nil {
return err
}

sc.user.updateInjector.send(ctx, imap.NewMessageMailboxesUpdated(
message.ID,
mboxIDs,
message.Flags.ContainsUnchecked(imap.FlagSeenLowerCase),
message.Flags.ContainsUnchecked(imap.FlagFlaggedLowerCase),
), true)
}

for _, mboxID := range mboxIDs {
mailbox, err := sc.connector.GetMailbox(ctx, mboxID)
if err != nil {
return err
}

sc.user.updateInjector.send(ctx, imap.NewMailboxUpdated(
mailbox.ID,
mailbox.Name,
), true)
}

return nil
}
3 changes: 1 addition & 2 deletions internal/state/actions.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,10 @@ func (state *State) actionDeleteMailbox(ctx context.Context, tx *ent.Tx, mboxID
return state.user.QueueOrApplyStateUpdate(ctx, tx, NewMailboxDeletedStateUpdate(mboxID.InternalID))
}

func (state *State) actionUpdateMailbox(ctx context.Context, tx *ent.Tx, mboxID imap.MailboxID, oldName, newName string) error {
func (state *State) actionUpdateMailbox(ctx context.Context, tx *ent.Tx, mboxID imap.MailboxID, newName string) error {
if err := state.user.GetRemote().UpdateMailbox(
ctx,
mboxID,
strings.Split(oldName, state.delimiter),
strings.Split(newName, state.delimiter),
); err != nil {
return err
Expand Down
2 changes: 1 addition & 1 deletion internal/state/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type Connector interface {
CreateMailbox(ctx context.Context, name []string) (imap.Mailbox, error)

// UpdateMailbox sets the name of the mailbox with the given ID to the given new name.
UpdateMailbox(ctx context.Context, mboxID imap.MailboxID, oldName, newName []string) error
UpdateMailbox(ctx context.Context, mboxID imap.MailboxID, newName []string) error

// DeleteMailbox deletes the mailbox with the given ID and name.
DeleteMailbox(ctx context.Context, mboxID imap.MailboxID) error
Expand Down
4 changes: 2 additions & 2 deletions internal/state/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -298,12 +298,12 @@ func (state *State) Rename(ctx context.Context, oldName, newName string) error {

newInferior := newName + strings.TrimPrefix(inferior, oldName)

if err := state.actionUpdateMailbox(ctx, tx, mbox.RemoteID, inferior, newInferior); err != nil {
if err := state.actionUpdateMailbox(ctx, tx, mbox.RemoteID, newInferior); err != nil {
return err
}
}

return state.actionUpdateMailbox(ctx, tx, result.MBox.RemoteID, oldName, newName)
return state.actionUpdateMailbox(ctx, tx, result.MBox.RemoteID, newName)
})
}

Expand Down

0 comments on commit 287db37

Please sign in to comment.