Skip to content

Fixing Race Condition on multiple commits to saveReceipts #1904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

brandonmaul
Copy link

@brandonmaul brandonmaul commented May 28, 2025

This fix solves a relatively rare race condition that happens when multiple commits are made to MXFileStore in quick succession to readReceipts. It manifests in an error such as in #1810

To solve this, we copy the entire RoomThreadedReceiptsStore for a room, and it's sub-values. We then use that "copy" to actually write to the store, and passing it along to the NSKeyedArchiver.

If there is a better approach to solving this please let us know, we're more than happy to evaluate it and we understand that this may not be the most efficient way of solving this issue.

Signed-off-by: Brandon Maul [email protected]

@brandonmaul
Copy link
Author

brandonmaul commented May 28, 2025

@pixlwave @stefanceriu @richvdh Asking for a review

@brandonmaul
Copy link
Author

brandonmaul commented May 29, 2025

For more context, here is roughly how you can reproduce the bug (that this change fixes)

let mxSession: MXSession = ...
let creds: MXCredentials = ...
guard mxSession.store is MXFileStore else {
  print("⚠️ Session's store is not MXFileStore.")
  return
}

var receipts: [MXReceiptData] = []
for i in 0...1000 {
  let receipt = MXReceiptData()
  receipt.userId      = creds.userId! + String(describing: i)
  receipt.threadId    = kMXEventTimelineMain
  receipt.eventId     = "$crashRepro\(UUID().uuidString)"
  receipt.ts          = UInt64(Date().timeIntervalSince1970 * 1000)
  
  receipts.append(receipt)
}

for i in 0...1000 {
  let room = "!reproRoom:\(creds.homeServer!)" + String(describing: i)
  for j in 0...1000 {
    let receipt = receipts[j]
    mxSession.store.storeReceipt(receipt, inRoom: room)
    mxSession.store.commit?()
  }
}

@AlexandreHauber
Copy link

Following 👀!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants