Page MenuHomePhabricator

[CommCoreModule] implement method to persist `olm` account
ClosedPublic

Authored by kamil on Dec 18 2023, 4:22 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Oct 18, 7:40 AM
Unknown Object (File)
Fri, Oct 18, 7:40 AM
Unknown Object (File)
Fri, Oct 18, 7:40 AM
Unknown Object (File)
Fri, Oct 18, 7:40 AM
Unknown Object (File)
Fri, Oct 18, 7:39 AM
Unknown Object (File)
Fri, Oct 18, 6:38 AM
Unknown Object (File)
Mon, Oct 14, 7:56 AM
Unknown Object (File)
Mon, Oct 14, 1:19 AM
Subscribers

Details

Summary

This code will be re-used a lot in methods introduced later in the stack - creating a separate function.

In all cases, this method will be called from a task invoked on the crypto thread, since tasks are sequential and invoked in order on one thread it should result in the same order on the database thread so there is no chance to override/miss some updates on crypto account.

Depends on D10370

Test Plan

Build the app, the rest is tested later in the stack.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Dec 18 2023, 6:47 AM
marcin requested changes to this revision.Dec 20 2023, 7:29 AM

Olm account retrieval, in-memory update and update state persistence should be a single atomic operation. This is what we should do:

  1. Have a function called like getCallAndUpdateOlmAccount.
  2. This function will schedule a job on database thread that gets data from SQLite calls olm account and finally persists it to the database. Probably we don't want to use crypto thread for any operation that requires to persist something afterwards.
This revision now requires changes to proceed.Dec 20 2023, 7:29 AM

I am afraid that I behaved too abruptly in my previous comment. Actually you code does not create any race condition provided that it is always executed from the crypto thread. Crypto thread ensures FIFO-ordered sequentiality of crypto operations so if at teh end of each crypto operation we will schedule update on the database thread that is also FIFO-ordered sequential we will be safe.

This revision is now accepted and ready to land.Dec 20 2023, 9:16 AM
native/cpp/CommonCpp/NativeModules/CommCoreModule.cpp
297 ↗(On Diff #34784)

We have to rename this method to sth like persistOlmData or persistCryptoModule since it actually persists both account and all session.

rename and make call blocking

This revision is now accepted and ready to land.Jan 2 2024, 5:17 AM