Skip to content
This repository has been archived by the owner on Dec 8, 2021. It is now read-only.

fix: workaround a libstdc++ on random_device #208

Merged
merged 4 commits into from
Mar 9, 2020

Conversation

coryan
Copy link
Member

@coryan coryan commented Mar 8, 2020

On fairly recent versions of libstdc++ the std::random_device class
fails when used by multiple threads:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94087

We workaround this problem by detecting the version of the standard C++
library, and if it is libstdc++ and it is a version after 20200128
then we use rdrand as the token value to initialize
std::random_device, which does not have the problem with multiple
threads.

Part of the work for googleapis/google-cloud-cpp#3437


This change is Reviewable

On fairly recent versions of libstdc++ the `std::random_device` class
fails when used by multiple threads:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94087

We workaround this problem by detecting the version of the standard C++
library, and if it is `libstdc++` and it is a version after 20200128
then we use `rdrand` as the token value to initialize
`std::random_device`, which does not have the problem with multiple
threads.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 8, 2020
@codecov
Copy link

codecov bot commented Mar 8, 2020

Codecov Report

Merging #208 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
+ Coverage   96.26%   96.28%   +0.01%     
==========================================
  Files          97       97              
  Lines        4421     4443      +22     
==========================================
+ Hits         4256     4278      +22     
  Misses        165      165
Impacted Files Coverage Δ
google/cloud/internal/random_test.cc 100% <100%> (ø) ⬆️
google/cloud/internal/random.cc 100% <100%> (ø) ⬆️
google/cloud/internal/random.h 100% <100%> (ø) ⬆️

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 c26a613...a35bc11. Read the comment docs.

@coryan coryan marked this pull request as ready for review March 8, 2020 04:01
google/cloud/internal/random.cc Outdated Show resolved Hide resolved
google/cloud/internal/random.cc Outdated Show resolved Hide resolved
google/cloud/internal/random_test.cc Outdated Show resolved Hide resolved
google/cloud/internal/random_test.cc Outdated Show resolved Hide resolved
google/cloud/internal/random_test.cc Outdated Show resolved Hide resolved
@coryan
Copy link
Member Author

coryan commented Mar 8, 2020

PTAL

google/cloud/internal/random.cc Outdated Show resolved Hide resolved
@coryan coryan merged commit f533447 into googleapis:master Mar 9, 2020
@coryan coryan deleted the workaround-libstdcpp-bug branch March 9, 2020 17:07
coryan added a commit to coryan/google-cloud-cpp-common that referenced this pull request Apr 1, 2020
The workaround put in place in googleapis#208 does not work with g++-9.3.1 on
Fedora:31. Despite this being a "newer" version of libstdc++, it does
not support `rdrand` as a token for `std::random_device` maybe because
the CPU specific tokens are not enabled.

For now, use `/dev/urandom` which seems to be supported on all Linux
distros. On other platforms we continue to use the default constructor
for `std::random_device`.

It might be possible to refine this and detect, at run-time, if `rdrand`
is supported, but that seems like an improvement that can wait a little
bit.
coryan added a commit that referenced this pull request Apr 1, 2020
The workaround put in place in #208 does not work with g++-9.3.1 on
Fedora:31. Despite this being a "newer" version of libstdc++, it does
not support `rdrand` as a token for `std::random_device` maybe because
the CPU specific tokens are not enabled.

For now, use `/dev/urandom` which seems to be supported on all Linux
distros. On other platforms we continue to use the default constructor
for `std::random_device`.

It might be possible to refine this and detect, at run-time, if `rdrand`
is supported, but that seems like an improvement that can wait a little
bit.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cpp-common#208)

On fairly recent versions of libstdc++ the `std::random_device` class
fails when used by multiple threads:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94087

We workaround this problem by detecting the version of the standard C++
library, and if it is `libstdc++` and it is a version after 20200128
then we use `rdrand` as the token value to initialize
`std::random_device`, which does not have the problem with multiple
threads.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cloud-cpp-common#272)

The workaround put in place in googleapis/google-cloud-cpp-common#208 does not work with g++-9.3.1 on
Fedora:31. Despite this being a "newer" version of libstdc++, it does
not support `rdrand` as a token for `std::random_device` maybe because
the CPU specific tokens are not enabled.

For now, use `/dev/urandom` which seems to be supported on all Linux
distros. On other platforms we continue to use the default constructor
for `std::random_device`.

It might be possible to refine this and detect, at run-time, if `rdrand`
is supported, but that seems like an improvement that can wait a little
bit.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cpp-common#208)

On fairly recent versions of libstdc++ the `std::random_device` class
fails when used by multiple threads:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94087

We workaround this problem by detecting the version of the standard C++
library, and if it is `libstdc++` and it is a version after 20200128
then we use `rdrand` as the token value to initialize
`std::random_device`, which does not have the problem with multiple
threads.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cloud-cpp-common#272)

The workaround put in place in googleapis/google-cloud-cpp-common#208 does not work with g++-9.3.1 on
Fedora:31. Despite this being a "newer" version of libstdc++, it does
not support `rdrand` as a token for `std::random_device` maybe because
the CPU specific tokens are not enabled.

For now, use `/dev/urandom` which seems to be supported on all Linux
distros. On other platforms we continue to use the default constructor
for `std::random_device`.

It might be possible to refine this and detect, at run-time, if `rdrand`
is supported, but that seems like an improvement that can wait a little
bit.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cpp-common#208)

On fairly recent versions of libstdc++ the `std::random_device` class
fails when used by multiple threads:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=94087

We workaround this problem by detecting the version of the standard C++
library, and if it is `libstdc++` and it is a version after 20200128
then we use `rdrand` as the token value to initialize
`std::random_device`, which does not have the problem with multiple
threads.
coryan added a commit to coryan/google-cloud-cpp that referenced this pull request Apr 24, 2020
…-cloud-cpp-common#272)

The workaround put in place in googleapis/google-cloud-cpp-common#208 does not work with g++-9.3.1 on
Fedora:31. Despite this being a "newer" version of libstdc++, it does
not support `rdrand` as a token for `std::random_device` maybe because
the CPU specific tokens are not enabled.

For now, use `/dev/urandom` which seems to be supported on all Linux
distros. On other platforms we continue to use the default constructor
for `std::random_device`.

It might be possible to refine this and detect, at run-time, if `rdrand`
is supported, but that seems like an improvement that can wait a little
bit.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants