-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Permute ThreadId
values
#110725
Permute ThreadId
values
#110725
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
024a763
to
6f46e86
Compare
I really don't think we should do this. I personally continue to think we should expose the underlying OS thread ID value. But if we're not going to do that, and we're making up our own values, is there some fundamental reason we should do this? |
That would be a breaking change, since
If I understood the Zulip discussion correctly, it was proposed to do this to stop users from thinking that the id indicated the creating order of threads. |
I on the other hand believe that this doesn't go far enough and the permutation should be seeded with a random value, similar to hashmap iteration order.
The documentation says
Using a randomized permutation ensures that nobody relies on the value by accident. E.g. storing things by threadid in a btreemap and then relying on iteration order, writing tests against specific values or things like that. |
I don't think we should be treating users as adversaries like this. |
-1. This is totally overkill to introduce a permutation function just to obfuscate the implementation detail that the thread IDs are distributed sequentially. I don't support anything more complicated than XOR-ing with some magic number. |
To be honest, the more I think about it, the less I like this. Relying on the exact value of an id is a bug, and permuting them won't help users fix it more easily. Bugs because of randomness are just as hard to debug as bugs relying on operation ordering. I'm keeping this PR up, in case T-libs decides to do this. Feel free to close it otherwise. |
Looking at all the comments above, it looks like there is not much enthusiasm for this change.
Doing that. |
As discussed on Zulip, users should never rely on the value of
ThreadId
, so apply a simple permutation when generating ids to make it appear random.permute
was ported from https://www.gkbrk.com/wiki/avalanche-diagram/ by @Pointerbender. It is not and does not need to be cryptographically secure.