-
Notifications
You must be signed in to change notification settings - Fork 191
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
Initialization of CONTEXT in timestamp::context::shared_context() not thread-safe #730
Comments
Thanks for pointing this out @tpunder! I think the important thing is for each UUID generated sequentially within the same timestamp interval (100ns for v1/v6 and 1ms for v7) to get a counter value that is larger than the previous. This is technically breakable under this scheme, but extremely unlikely. For v7 UUIDs it's recommended we randomize the counter each millisecond. I'm not sure if we'll want to do that, but should come up with some scheme to occasionally shuffle the counter. So I think there is an issue here we need to fix. The solution to it will probably be whatever we come up with for v7 UUIDs using counters. |
The initialization of
CONTEXT
inshared_context()
is not thread-safe. Multiple threads callingshared_context()
can end up using an un-initializedCONTEXT
before the thread that successfully callscompare_exchange
can fully initializeCONTEXT
.uuid/src/timestamp.rs
Lines 372 to 392 in cefc353
Here is an example that demonstrates the problem:
Here are a few runs from an Macbook Pro M1 (arm64). You can see some of the threads are starting from the un-initialized value of 0 and counting from there before the thread that successfully runs the
compare_exchange
is able to actually initializedCONTEXT
with a random value:Here are some runs from an Intel Core i7 (x86_64):
The text was updated successfully, but these errors were encountered: