Skip to content

Conversation

dhardy
Copy link
Member

@dhardy dhardy commented Apr 28, 2020

This is @josephlr's fix (3) from #968.

Personally I see 2-5% performance penalty vs Joseph's ~16% (same benchmarks; in my case using Xeon E3-1231 aka Haswell with Fedora 31). Either way, real-world impact will usually be much lower due to the tight loops in the benchmarks.

CC @nathdobson @burdges @bjorn3.

@@ -55,8 +55,8 @@ const THREAD_RNG_RESEED_THRESHOLD: u64 = 1024 * 64;
/// [`StdRng`]: crate::rngs::StdRng
#[derive(Copy, Clone, Debug)]
pub struct ThreadRng {
// inner raw pointer implies type is neither Send nor Sync
rng: NonNull<ReseedingRng<Core, OsRng>>,
// type if neither Send nor Sync
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be is?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

@dhardy dhardy merged commit f58a6b1 into rust-random:0.7 May 12, 2020
@dhardy
Copy link
Member Author

dhardy commented May 12, 2020

Darn, appears I pushed to the wrong destination and accidentally merged (no protection on this branch).

@vks do you approve or shall I revert?

@vks
Copy link
Collaborator

vks commented May 12, 2020

It's fine with me, I just did not check whether this actually fixes the use-after-free.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants