rand: replace lazy_static with std::sync::Once

[bpowers]

This replaces the use of lazy_static with mutable statics. I'm pretty new to Rust, would love any feedback!

fixes #706

I agree to license my contributions to each file under the terms given at the top of each file I changed.

[briansmith]

@bpowers, thanks for writing this! I'm going to review this PR line-by-line, then I'll explain the mistake I made in asking for this change.

[briansmith]

So, I definitely like getting rid of the lazy_static dependency.

However, I did not realize that we'd have to add new uses of unsafe in the code by switching to a raw static mut, as I'd never used static mut before. In hindsight, this is obvious, but it just hadn't occured to me.

Definitely we shouldn't put new unsafe uses in ring::rand; either we should implement this in polyfill or we should continue depending on a third-party crate, be it lazy_static or one of the simpler alternatives. I'm not sure which course of action is best. WDYT?

[briansmith]

It took me a while to understand that you factored out get_random_file() as a function because you wanted to limit access to FILE to avoid the potential safety issues that are inherent in static mut. It is important to document this so that people don't refactor the code to remove this extra level of safety.

Normally we document such things, especially things that use unsafe, by (1) factoring the code out into its own module under the polyfill submodule, and (2) writing comments.

In this case, that would basically be re-implementing lazy_static and other similar crates. We could do it a lot simpler than lazy_static since this code doesn't need no_std support and it doesn't need to work on old versions of Rust, so we could avoid the version_check dependency that lazy_static has. However, I think there are probably better alternative crates to lazy_static that we could/should use instead of implementing it ourselves in polyfill. We should evaluate those alternatives vs. lazy_static and vs. implementing it ourselves.

If we are going to maintain our own implementation of this, then (1) it should be a polyfill submodule, and (2) we should document why we're implementing it, e.g. by documenting the issues that other crates have, noting that build speed is faster (almost definitely), noting that we don't have to track third-party dependency (lazy_static and version_check, compared to lazy_static), avoiding one heap allocation, simpler code, and any other benefits.

[briansmith]

I think _ONCE would be a better naming convention than _INIT.

[briansmith]

Let's minimize the scope of unsafe { }. In this case, I think the unsafe {} should go inside the body of the closure passed to FILE_INIT.call_once, something like:

FILE_INIT.call_once(|| {
    let file = std::fs::File::open(RANDOM_PATH).ok();
    unsafe { FILE = file; }
});
unsafe {
    &FILE
}

Similarly, the scope of RANDOM_PATH should be minimized, by moving it into the closure.

[briansmith]

All the issues I pointed out above also apply here.

[briansmith]

@bpowers Thank you very much for the PR. I really appreciate your help here. I'm going to close this PR based on the discussion above; I will explain it in the issue that this PR was attempting to resolve.

[bobby-stripe]

@briansmith sorry I didn't respond sooner -- I agree with what you wrote above and in the issue about lazy_static. SGTM!