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

avoid using rand.Source #3046

Merged
merged 3 commits into from
Feb 18, 2021
Merged

avoid using rand.Source #3046

merged 3 commits into from
Feb 18, 2021

Conversation

marten-seemann
Copy link
Member

@marten-seemann marten-seemann commented Feb 17, 2021

Blocked by #3045, which is blocked by CircleCI. @circleci, thanks for slowing down my development workflow!

Fixes #3044.

The alternative would be to use one rand.Source for all connections, but then we'd have to deal with it not being thread-safe. Reading 4 bytes from crypto/rand every 10000 packets shouldn't be too expensive, so we can just use cryptographic random here.

@codecov
Copy link

codecov bot commented Feb 17, 2021

Codecov Report

Merging #3046 (03fe636) into master (c72f05a) will decrease coverage by 0.03%.
The diff coverage is 66.67%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3046      +/-   ##
==========================================
- Coverage   86.20%   86.17%   -0.03%     
==========================================
  Files         131      132       +1     
  Lines        9356     9352       -4     
==========================================
- Hits         8065     8059       -6     
- Misses        935      936       +1     
- Partials      356      357       +1     
Impacted Files Coverage Δ
internal/ackhandler/packet_number_generator.go 100.00% <ø> (+3.57%) ⬆️
internal/utils/rand.go 62.50% <62.50%> (ø)
conn_id_manager.go 93.18% <100.00%> (-0.30%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c72f05a...03fe636. Read the comment docs.

@marten-seemann marten-seemann merged commit bfad411 into master Feb 18, 2021
@marten-seemann marten-seemann deleted the avoid-rand-source branch February 18, 2021 03:14
@aschmahmann aschmahmann mentioned this pull request May 14, 2021
71 tasks
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.

rand.Source consumes a lot of memory
2 participants