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

Thread Safety? #61

Closed
whatamithinking opened this issue Sep 25, 2022 · 4 comments
Closed

Thread Safety? #61

whatamithinking opened this issue Sep 25, 2022 · 4 comments

Comments

@whatamithinking
Copy link

Since this package is on PyPI and shows up in search engine results, I just wanted to point out that it does not appear to be threadsafe.

The global variable _last_v7_timestamp is getting modified but does not have a lock around it.

@whatamithinking whatamithinking changed the title Threadsafety? Thread Safety? Sep 25, 2022
@jcary741
Copy link

jcary741 commented Oct 6, 2022

I am not a maintainer of this repository, but I have to ask: does thread safety matter for this project? This is a python library, so as far as I understand the global interrupt lock essentially prevents code from executing in parallel. Is that incorrect?

@whatamithinking
Copy link
Author

The GIL prevents true parallelism, but code execution can still start and stop anywhere.

The code in the uuid7() function, reproduced below, could have one thread reach the line nanoseconds = _last_v7_timestamp + 1 and pause and then another thread reach the same line, pause, and switch back to the first.

In that event, both threads could end up with the same timestamp and appear in the same order even though thread 1 came before thread 2.

This may seem contrived, but if you are making these uuid7() calls in tight loops on multiple threads it can happen. If clock precision on the OS does not reach the nanosecond level, this issue may occur more often.

If the UUIDs are out of order, it may break downstream code which tries to do basic duplicate checking by skipping UUIDs which are less than the last cached value from a given source system/process where it is assumed they will always be in lexicographic order.

def uuid7() -> UUID:
    global _last_v7_timestamp

    nanoseconds = time.time_ns()
    if _last_v7_timestamp is not None and nanoseconds <= _last_v7_timestamp:
        nanoseconds = _last_v7_timestamp + 1
    _last_v7_timestamp = nanoseconds
    
    ...code continues...

@oittaa
Copy link
Owner

oittaa commented Oct 19, 2022

If you are using threads and you need to guarantee the order of the generated UUIDs, you should use locks around uuid7() just like you would use with the official UUIDv1 implementation.

@whatamithinking
Copy link
Author

@oittaa you are right. i had not noticed that before. they are mutating global state in that module as well. i guess that makes sense to avoid the overhead of a lock unless you need it.

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

3 participants