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 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.
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 lockstore.create_store_lock("oidc_session_refresh_lock".to_owned(), lock_value.clone());
whereas theSyncService
one controlsolm_machine.store().create_store_lock("cross_process_lock".to_owned(), lock_value);
.Independent to this, creating a
NotificationClient
implicitly creates a lock oncross_process_lock
with the value LOCK_ID 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:
enableCrossProcessRefreshLock
andwithCrossProcessLock
to better reflect which locks you are talking about e.genableOIDCCrossProcessLock
andwithEncryptionCrossProcessLock
would be much clearer IMO.NotificationClient
to be changed at the FFI layer, so the app can avoid making bad choices.appIdentifier
whereas the other calls itprocessId
.