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.

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

fix: random state reusage #326

Merged
merged 5 commits into from
Nov 12, 2024
Merged

fix: random state reusage #326

merged 5 commits into from
Nov 12, 2024

Conversation

gferon
Copy link
Collaborator

@gferon gferon commented Nov 8, 2024

This includes the fix whisperfish/presage#292 in particular fixing the long-standing issue #234.

Also adjust usage of presage::Store API which is now asynchronous. One possibly negative side-effect is that the UI looks up in the contact names cache in read-only fashion, and only the async part of gurk can populate the cache using the store.

Also fix clippy warnings.

and adjust usage of presage::Store API which is now asynchronous.
@gferon gferon requested a review from boxdot November 8, 2024 14:33
@gferon gferon mentioned this pull request Nov 8, 2024
@hrdl-github
Copy link
Contributor

@boxdot
Copy link
Owner

boxdot commented Nov 9, 2024

@hrdl-github I don't see why cloning the rng is a problem. The cloned rng is neither Send nor Sync, so it can be only used on the same thread it was cloned on. Also the thread_rng function actually just clones the thread local rng variable. It seems to me that this code is equivalent.

edit: Ok, the difference is that in presage StdRng is used and in your branch a ThreadRng.

@boxdot
Copy link
Owner

boxdot commented Nov 9, 2024

This explains a lot

use rand::rngs::StdRng;
use rand::{RngCore, SeedableRng};

fn main() {
    let mut rng = StdRng::from_seed([0; 32]);
    let mut rng2 = rng.clone();

    println!("{:?}", rng.next_u64());
    println!("{:?}", rng2.next_u64());
}

Output

6050961064690644123
6050961064690644123

@boxdot
Copy link
Owner

boxdot commented Nov 9, 2024

Thread rng uses (current impl in rand) the os rng. But more important, it uses always the same thread local instance. So, on the same thread cloning this rng is fine, different clones still point to the same instance. However, the StdRng (current impl) is a chacha rng. Cloning it just clones its state. And different clones produce the same sequence of random numbers.

@boxdot boxdot changed the title Update Presage fix: random state reusage Nov 11, 2024
@hrdl-github hrdl-github mentioned this pull request Nov 11, 2024
@boxdot boxdot merged commit 2a1f5af into master Nov 12, 2024
16 checks passed
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.

3 participants