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

doc: crypto.randomBytes does not block when async #14993

Merged
merged 1 commit into from
Aug 25, 2017

Conversation

sam-github
Copy link
Contributor

It may not return random bytes right away, but when called
asynchronously it will not block.

Checklist
Affected core subsystem(s)

doc

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations. labels Aug 23, 2017
Copy link
Member

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

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

LGTM with a suggestion.

@@ -1717,7 +1717,8 @@ console.log(
`${buf.length} bytes of random data: ${buf.toString('hex')}`);
```

The `crypto.randomBytes()` method will block until there is sufficient entropy.
The `crypto.randomBytes()` method will not generate bytes until there is
Copy link
Member

@bnoordhuis bnoordhuis Aug 23, 2017

Choose a reason for hiding this comment

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

s/generate bytes/complete/?

@@ -1717,7 +1717,8 @@ console.log(
`${buf.length} bytes of random data: ${buf.toString('hex')}`);
```

The `crypto.randomBytes()` method will block until there is sufficient entropy.
The `crypto.randomBytes()` method will not complete until there is
Copy link
Contributor

@mscdex mscdex Aug 23, 2017

Choose a reason for hiding this comment

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

Isn't this only the case for the async version of this function (e.g. when a callback is passed)? How does it not block when the sync version is invoked and there is not sufficient entropy at that time? I think we should be explicit here about this difference.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its the case for both versions. If its sync, it won't complete. If it's async, it won't complete. Can you suggest some other wording? I think the current wording applies equally to both, and the previous wording applied only to the sync version.

It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: nodejs#14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@sam-github sam-github force-pushed the crypto-does-not-block branch from 19a79ba to 68321b5 Compare August 25, 2017 17:06
@sam-github sam-github merged commit 68321b5 into nodejs:master Aug 25, 2017
ghost pushed a commit to ayojs/ayo that referenced this pull request Aug 30, 2017
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: nodejs/node#14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
ghost pushed a commit to ayojs/ayo that referenced this pull request Aug 30, 2017
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: nodejs/node#14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@sam-github sam-github deleted the crypto-does-not-block branch September 7, 2017 20:51
MylesBorins pushed a commit that referenced this pull request Sep 10, 2017
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: #14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Sep 10, 2017
MylesBorins pushed a commit that referenced this pull request Sep 12, 2017
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: #14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins pushed a commit that referenced this pull request Sep 20, 2017
It may not return random bytes right away, but when called
asynchronously it will not block.

PR-URL: #14993
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Sep 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crypto Issues and PRs related to the crypto subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants