nix icon indicating copy to clipboard operation
nix copied to clipboard

Use portable C++ Pseudorandom number generator

Open Ericson2314 opened this issue 1 year ago • 10 comments

Avoid calls to rand / srand / random / srandom and other things which are not portable to Windows

Originally posted by @edolstra in https://github.com/NixOS/nix/pull/8901#discussion_r1568529190

Ericson2314 avatar Apr 17 '24 16:04 Ericson2314

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/nix-on-windows/1113/109

nixos-discourse avatar Apr 17 '24 17:04 nixos-discourse

I see that random() is called in two places:

src/libstore/unix/gc.cc:44: Path tempLink = fmt("%1%.tmp-%2%-%3%", link, getpid(), random()); src/libstore/unix/optimise-store.cc:228: Path tempLink = fmt("%1%/.tmp-link-%2%-%3%", realStoreDir, getpid(), random());

Should we create a PRNG before every call? Or create a PRNG in initNix that somehow gets passed to the functions that need it?

RCoeurjoly avatar Apr 19 '24 15:04 RCoeurjoly

@RCoeurjoly good question! When in doubt, it is better to avoid global state.

I think for the gc one, it would be good to put the PRNG state in IndirectRootStore, and since LocalStore is a subclass, the optimise-store one can use that too.

Thanks for looking into this!

Ericson2314 avatar Apr 19 '24 17:04 Ericson2314

I see another random() call in - src/libstore/unix/build/derivation-goal.cc:826 too

dottharun avatar Apr 19 '24 21:04 dottharun

That can also use a per-store PRNG

Ericson2314 avatar Apr 19 '24 23:04 Ericson2314

The problem is that random() in gc.cc:44 is called by makeSymlink(const Path & link, const Path & target), a free function, so there is no way to access a per-store PRNG.

I was thinking of the following:

Initialize the PRNG in initNix, as it is done now with srandom(). Use a class with static members.

Use this class wherever is needed.

Thoughts?

RCoeurjoly avatar Apr 20 '24 14:04 RCoeurjoly

IndirectRootStore Mix-in seems to have a clear purpose of providing its two methods

  • do we have to use it for PRNG just for its structure?
  • maybe some other class in libutil/ as seeding needs to done only once?

dottharun avatar Apr 20 '24 20:04 dottharun

@RCoeurjoly After #10556 it will not be a free function

@dottharun And the PRNG will be for one the IndirectRootStore methods; the indirect root symlinks not colliding in the directory is part of the spec! :)

Ericson2314 avatar Apr 22 '24 14:04 Ericson2314

After https://github.com/NixOS/nix/pull/10556 it will not be a free function

If anyone wants to take this issue, they should feel free to do just redo that part of #10556 in their PR. That is better than waiting for my PR.

Ericson2314 avatar Apr 22 '24 15:04 Ericson2314

https://github.com/NixOS/nix/pull/10556 is now merged, so this should be a bit easier.

Ericson2314 avatar May 06 '24 14:05 Ericson2314