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

Fallback to the global allocator when the TLS var has been deinitialized. #154

Closed
ticki opened this issue May 23, 2017 · 5 comments
Closed

Comments

@ticki
Copy link

ticki commented May 23, 2017

Currently you (sometimes) get a panick if you use rand::random() in a TLS dtor. This is because the RNG state TLS var can have been deinitialized and thus unavailable. Ideally, rand should check if the TLS var is available (https://doc.rust-lang.org/nightly/std/thread/struct.LocalKey.html#method.state), and if not, fallback to the global RNG state.

@alexcrichton alexcrichton added the X-bug Type: bug report label Jun 14, 2017
@pitdicker
Copy link
Contributor

There are no stable methods to do this yet, see rust-lang/rust#27716. try_with seems to be what we need.

With 'global RNG state you probably mean OsRng?

@dhardy
Copy link
Member

dhardy commented Jan 22, 2018

Question: why are you using random from a destructor? In general one must be very careful in destructors.

Besides, we don't have a "global RNG state".

@pitdicker
Copy link
Contributor

I suppose it has become a good time to work on this now that try_from is stabilised on nightly rust-lang/rust#48585.

Replacing with with try_with in thread_rng() is not that hard. We would need to add an enum in ThreadRng to select between its standard PRNG and EntropyRng (assuming that is the fallback). We could also freshly initialize some other PRNG, or just return a stream of zero's. In all cases it would mean more code to execute for next_u32 and next_u64, where every extra instruction and branch causes noticeable slowdown.

Or is using thread_rng in destructors just not a good idea, maybe even a logic bug? Especially when a thread is broken down? In that case a panic as we have now is more appropriate.

I suppose I can see one use case: filling memory or files with random bits before closing/deallocation (although I don't see the advantage over zeroing).

Should we fix this?

@dhardy
Copy link
Member

dhardy commented Mar 4, 2018

I'm not really sure that we should. But I suggest we leave this issue open. It's not a bug though, more a feature.

@dhardy dhardy added enhancement and removed X-bug Type: bug report labels Mar 4, 2018
@dhardy
Copy link
Member

dhardy commented May 30, 2018

This is a year old now, and doesn't appear to have any other interested parties. Besides which, usage of a random number generator in a destructor is probably a poor design, given that one cannot guarantee destructors run anyway (e.g. if you wanted to write over password memory, better to do it before destruction). So, lets close.

@dhardy dhardy closed this as completed May 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants