[SERVER-72366] Pass random seed to `srand` for unit tests Created: 22/Dec/22 Updated: 03/Jan/23 Resolved: 03/Jan/23 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Bug | Priority: | Major - P3 |
| Reporter: | Pierlauro Sciarelli | Assignee: | Billy Donahue |
| Resolution: | Duplicate | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||
| Assigned Teams: |
Service Arch
|
||||||||||||||||
| Operating System: | ALL | ||||||||||||||||
| Participants: | |||||||||||||||||
| Description |
|
In order to generate random-like numbers, srand is usually initialized to some distinctive runtime value. For example, in mongod main and mongos_main the seed is always based on the current timestamp XOR some random address. In unit tests the generator is not seeded, so it's very difficult to generate real random sequences. There are a lot of unit tests relying on `std::rand()` (aka `rand()`) expecting to get nondeterministic behavior but that's not the case as shown by this unit test executed 3 times:
|
| Comments |
| Comment by Billy Donahue [ 03/Jan/23 ] |
|
This will be obsoleted by But even PseudoRandom-based generators need injectable seeds. |
| Comment by Billy Donahue [ 23/Dec/22 ] |
|
antonio.fuschetto@mongodb.com I haven't heard about security defects from <random>. |
| Comment by Antonio Fuschetto [ 23/Dec/22 ] |
|
I've extensively used Synopsys Coverity in the past (for another company) and I recall that there was a default rule such that the use of the standard functions to generate random numbers causes critical security defects. Coverity's suggestion was to replace them with the Boost implementation. I don't know if this suggestion is still valid today, this implementation is probably included in the C+11, C14 or C+17 standard libraries today. |
| Comment by Billy Donahue [ 22/Dec/22 ] |
|
Unit tests are currently relying on the default srand seed of 1. I don't think we should change this. The existing behavior has a benefit of making repeatable unit tests. If we want to change this we should do it in a structured way. We have platform/random.h as a place to hang such coordinating code. It's also worth saying that std::rand shouldn't be used at all, and the C++11 <random> facilities are better. Every call to std::rand in server code or in tests is a bug that should be fixed. |