Skip to content

Commit

Permalink
fix: Add WaitGroup for process() goroutine completion
Browse files Browse the repository at this point in the history
We need to ensure that the User.process() goroutine has finished
executing before we serialize the queue to disk. It is currently
possible that the goroutine is able to pop operations during the queue's
closure. This in turn can result in a race condition when accessing the
connection metadata.
  • Loading branch information
LBeernaertProton committed Jun 13, 2022
1 parent bf2f1c3 commit b11b5e4
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 0 deletions.
7 changes: 7 additions & 0 deletions internal/remote/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ type User struct {
closedLock sync.RWMutex

connMetadataStore connMetadataStore

// processWG is used to ensure we wait until the process goroutine has finished executing after we close the queue.
processWG sync.WaitGroup
}

// newUser constructs a new user with the given (IMAP) credentials.
Expand All @@ -61,6 +64,7 @@ func newUser(userID, path string, conn connector.Connector) (*User, error) {
// send connector updates along to the mailserver.
go user.forward(conn.GetUpdates())

user.processWG.Add(1)
// process remote operations on the operation queue.
go user.process()

Expand All @@ -79,6 +83,9 @@ func (user *User) Close() error {
return fmt.Errorf("failed to close queue: %w", err)
}

// Wait until any remaining operations popped by the process go routine finish executing
user.processWG.Wait()

if user.lastOp != nil {
ops = append([]operation{user.lastOp}, ops...)
}
Expand Down
2 changes: 2 additions & 0 deletions internal/remote/user_queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ var ErrQueueClosed = errors.New("the queue is closed")
// TODO: What should we do with operations that failed to execute due to auth reasons?
// We might want to save them somewhere so we can try again after the user has logged back in.
func (user *User) process() {
defer user.processWG.Done()

for {
op, ok := user.popOp()
if !ok {
Expand Down

0 comments on commit b11b5e4

Please sign in to comment.