matrix-org / matrix-org/matrix-rust-sdk

Locks in the FFI layer are confusing

Open
#3,336 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2.3k
Forks
500
Avg merge
1d 16h
Merged PRs (30d)
106

Description

There's two distinct functions to control locks:
- `ClientBuilder.enableCrossProcessRefreshLock(processName, ...)`
- `SyncService.withCrossProcessLock(processName)`

Despite having similar names, they control different independent locks. The `ClientBuilder` variant controls the lock `store.create_store_lock("oidc_session_refresh_lock".to_owned(), lock_value.clone());` whereas the `SyncService` one controls `olm_machine.store().create_store_lock("cross_process_lock".to_owned(), lock_value);`.

Independent to this, creating a `NotificationClient` implicitly creates a lock on `cross_process_lock` with the value [LOCK_ID](https://github.com/matrix-org/matrix-rust-sdk/blob/f7329c71bbf518869a7d96e89114dfe297dffdab/crates/matrix-sdk-ui/src/notification_client.rs#L243) which is `"notifications"`.

This is all very confusing and very very footgunny, because if 2 independent processes create NotificationClient instances they will accidentally use the same lock value!

Proposed changes:
- Rename `enableCrossProcessRefreshLock` and `withCrossProcessLock` to better reflect which locks you are talking about e.g `enableOIDCCrossProcessLock` and `withEncryptionCrossProcessLock` would be much clearer IMO.
- Allow the lock value used in `NotificationClient` to be changed at the FFI layer, so the app can avoid making bad choices.
- Provide guidance and consistency in the terminology around the "lock value". One calls it an `appIdentifier` whereas the other calls it `processId`.

Contributor guide

Open the contributing guide

Research direction

Start by comparing the FFI functions `ClientBuilder.enableCrossProcessRefreshLock` and `SyncService.withCrossProcessLock` with the lock creation sites named in the issue. Inspect `crates/matrix-sdk-ui/src/notification_client.rs`, especially the `LOCK_ID` usage, and trace how the lock value reaches the FFI layer. Done means the lock APIs are clearly named, NotificationClient's value can be configured through FFI, and `appIdentifier` and `processId` terminology is consistent.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.