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

only seed math/rand once #21710

Merged
merged 1 commit into from
Jun 13, 2019
Merged

only seed math/rand once #21710

merged 1 commit into from
Jun 13, 2019

Conversation

jbardin
Copy link
Member

@jbardin jbardin commented Jun 13, 2019

Re-seeding the PRNG every time only serves to make the output an
obfuscated timestamp. On windows with a low clock resolution, this
manifests itself by outputting the same value on calls within the
minimum time delta of the clock.

Fixes #21550

Re-seeding the PRNG every time only serves to make the output an
obfuscated timestamp. On windows with a low clock resolution, this
manifests itself by outputting the same value on calls within the
minimum time delta of the clock.
@jbardin jbardin requested a review from a team June 13, 2019 01:42
@jbardin jbardin merged commit edd005c into master Jun 13, 2019
@jbardin jbardin deleted the jbardin/acctest-random branch June 13, 2019 12:34
@apparentlymart
Copy link
Contributor

This fix seems reasonable, but got me thinking more: we have a few different things in Terraform and in providers that generate random numbers, and this seems to be using the default shared generator that other code might also be reseeding.

Should we give this package its own generator instance so that it can be sure it isn't going to get reseeded in a bad way by some other code? (This feels similar to our principle of avoiding using the default HTTP client because other libraries that we load might/did do weird things to it.)

@jbardin
Copy link
Member Author

jbardin commented Jun 13, 2019

I was thinking the same thing, but didn't create a new source since it's actually a bit awkward to make s locked source, and figured I could find one in our own packages to use instead.

Since this is the standard way of using the default rand.Source, any package seeding things incorrectly is what should be creating a new source, however you're right that we should probably protect this from badly behaved packages anyway.

@ghost
Copy link

ghost commented Jul 25, 2019

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Jul 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

helper/acctest/random module returns duplicate values (from Windows Git Shell/MINGW64)
3 participants