Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Element-R: one-time-keys processing is racy #25214

Closed
richvdh opened this issue Apr 26, 2023 · 1 comment · Fixed by matrix-org/matrix-js-sdk#3327
Closed

Element-R: one-time-keys processing is racy #25214

richvdh opened this issue Apr 26, 2023 · 1 comment · Fixed by matrix-org/matrix-js-sdk#3327
Assignees
@florianduros
Labels
A-Element-R Issues affecting the port of Element's crypto layer to Rust T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Apr 26, 2023

I sometimes see the following being logged:

19:35:03.528 OutgoingRequestProcessor.ts:109 rust-crypto: successfully made HTTP request: POST /_matrix/client/v3/keys/upload
19:35:03.532 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 0 -> 50.
19:35:03.578 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 50 -> 0.
19:35:03.751 OutgoingRequestProcessor.ts:109 rust-crypto: successfully made HTTP request: POST /_matrix/client/v3/keys/signatures/upload
19:35:03.816 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 0 -> 50.
19:35:03.853 OutgoingRequestProcessor.ts:109 rust-crypto: successfully made HTTP request: POST /_matrix/client/v3/keys/upload
19:35:03.854 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 50 -> 100.
19:35:03.894 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 100 -> 50.
19:35:34.057 matrix_sdk_crypto_js.js:6371 DEBUG crates/matrix-sdk-crypto/src/olm/account.rs:614 Updated uploaded one-time key count 50 -> 100.

Note the flapping of the one-time-key-count (and the fact we end up at 100).

I think what's happening is that the /upload request is being made in parallel with a sync request:

  • We start a /sync request
  • We do an /upload request, which completes, and sets the count to 50
  • We process the response to the /sync request, which has a count of 0
  • That resets the count, and we do another /upload request

... I think all that is needed is to make processSyncResponse await when it calls processKeyCounts.
@richvdh richvdh added T-Defect A-Element-R Issues affecting the port of Element's crypto layer to Rust labels Apr 26, 2023
@richvdh
Copy link
Member Author

richvdh commented Apr 26, 2023

/cc @florianduros

@florianduros florianduros mentioned this issue Apr 27, 2023
Merged
3 tasks
@florianduros florianduros self-assigned this Apr 27, 2023
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Jul 31, 2023
@su-ex
Merge tag 'v25.1.0' into sc
399467c 
* Deprecate MatrixClient::resolveRoomAlias ([\matrix-org#3316](matrix-org#3316)).
* add client method to remove pusher ([\matrix-org#3324](matrix-org#3324)). Contributed by @kerryarchibald.
* Implement MSC 3981 ([\matrix-org#3248](matrix-org#3248)). Fixes element-hq/element-web#25021. Contributed by @justjanne.
* Added `Room.getLastLiveEvent` and `Room.getLastThread`. Deprecated `Room.lastThread` in favour of `Room.getLastThread`. ([\matrix-org#3321](matrix-org#3321)).
* Element-R: wire up device lists ([\matrix-org#3272](matrix-org#3272)). Contributed by @florianduros.
* Node 20 support ([\matrix-org#3302](matrix-org#3302)).
* Fix racing between one-time-keys processing and sync ([\matrix-org#3327](matrix-org#3327)). Fixes element-hq/element-web#25214. Contributed by @florianduros.
* Fix lack of media when a user reconnects ([\matrix-org#3318](matrix-org#3318)).
* Fix TimelineWindow getEvents exploding if no neigbouring timeline ([\matrix-org#3285](matrix-org#3285)). Fixes element-hq/element-web#25104.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@florianduros florianduros

Labels
A-Element-R Issues affecting the port of Element's crypto layer to Rust T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix racing between one-time-keys processing and sync matrix-org/matrix-js-sdk
2 participants
@richvdh @florianduros