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

Fix thread issue with UUID generation #167

Merged
merged 2 commits into from
Apr 6, 2020
Merged

Conversation

svwilliams
Copy link
Contributor

The original Boost random UUID generator is thread-safe, but it was very slow. Some time ago, I replaced the generator code with equivalent logic using the STL random library to improve performance. But had the downside of not being thread-safe. This manifests itself occasionally with different constraints having the same UUID.

This PR includes:

  • A mutex lock on the random number generator to handle calls from multiple threads correctly.
  • A slight refactor of how the random bits are copied into the UUID bytes in an attempt to minimize the lock time.

fuse_core/src/uuid.cpp Outdated Show resolved Hide resolved
int i = 0;
auto random_value = distibution(generator);
for (UUID::iterator it = u.begin(); it != u.end(); ++it, ++i)
uint64_t random1;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please pay extra attention to this refactor.

My interpretation of the original code is that a random number of N bytes is generated. Inside the loop, each random byte is copied into the next open slot in the UUID. When you run out of random bytes, generate N more bytes.

The refactor relies on the uint8_t type in the UUID and the uint64_t type generated by the random number generator being the expected sizes.

That seems like a strong guarantee in C++:

unsigned integer type with width of exactly 8, 16, 32 and 64 bits respectively
(provided only if the implementation directly supports the type)

https://en.cppreference.com/w/cpp/types/integer

Copy link

@needphpsql needphpsql left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good to me.

@svwilliams svwilliams merged commit ebaf511 into devel Apr 6, 2020
@svwilliams svwilliams deleted the thread-safe-uuids branch April 6, 2020 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants