Limit OsRng to a single file handle when reading from a file

[dhardy]

Fix #234.

I'm not entirely happy with the code; it uses unwrap a lot and requires use of unsafe to access the mutex, and it also requires Rust 1.22; anything older gets:

error[E0493]: destructors in statics are an unstable feature

lazy_static gets around this by using a Box cast to a pointer which never gets freed.

The reasons I didn't use lazy_static are:

• it would be difficult to encapsulate the logic in ReadRng since lazy_static doesn't allow a constructor or argument to be passed
• it would still require the mutex to safeguard access from other threads

[burdges]

I'd think the Once and outer Option can be eliminated once const fn advances enough to cover Mutex::new, but that'll be quite a while.

[dhardy]

As it is this requires using a very recent compiler compared to the 1.15 we support now, so I'm hesitant adding it for that reason. Rand v0.5 will have quite a few changes anyway though so may be a good time to update the compiler.

Thanks for taking a look.

[dhardy]

Rebased. I plan to merge this with the bump to the minimum Rust version, and probably with all the unwrap usage (I don't see a better alternative). Any comments?

[pitdicker]

When you made this PR I was curious how often reading from /dev/urandom is really needed anymore, see rust-lang/rfcs#2152 (comment).

How many distributions support old(er) pre-3.17 Linux kernel versions?

• Debian Jessie uses 3.16 and receives security support until may 2018.
• Ubuntu 14.04 (Trusty Tar) uses 3.13, and is supported until April 2019.
• RHEL 6. uses kernel 2.6.32, supported until November 2020.
• RHEL 7.4 kernel 3.10, but is said to support getrandom (https://access.redhat.com/blogs/766093/posts/3050871).

So still necessary on some old but still supported Linux distro's.

MacOS should switch to the same method as iOS, and not read from /dev/urandom.

Then there are Dragonfly BSD and NetBSD that don't have a better interface, and Redox. Not sure how Redox handles file descriptors, and maybe they are willing to provide another interface?

If I forget about the BSD's for a moment, I have a bit mixed feelings about requiring a newer compiler version and extra complexity to support an edge case on older platforms. I am not saying I disagree, just pointing out when this code is going to get used.

[pitdicker]

Is it possible to return an new error instead of unwrapping? For the rest I don't feel confident to review, do you want to cc someone?

[dhardy]

@burdges are you happy enough reviewing this code? (I guess you already did; it hasn't changed.)

@pitdicker yes, but all 5 unwraps would be code logic errors if they failed, and consequently OsRng would be permanently unavailable. I don't feel like panic from unwrap is such a bad option if somehow the logic breaks; it at least forces visibility of the issue.

Note: the Servo devs had a real problem here frequently running out of file handles when running tests.

[dhardy]

@asajeffrey @jdm review request

[cuviper]

The rustc bump becomes doubly painful when you have rand-0.3.22 now depending on rand-0.4.

[dhardy]

This was merged into master, which will be released as 0.5 eventually, not 0.4. There are several breaking changes between 0.4 and 0.5 so I don't anticipate the same kind of compatibility release (possibly there will be partial compatibility by opting into a feature).

[cuviper]

@dhardy Ah, sorry! I misunderstood the errors we were getting from rayon using rand-0.3, which now uses rand-0.4. I actually was previously aware that rand-0.4 required rust 1.15, but I guess I forgot this and then jumped to the wrong conclusion when searching the reason for this new break.