-
Notifications
You must be signed in to change notification settings - Fork 58
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
parse_time() is not threadsafe #54
Comments
This is what's keeping me from using this (otherwise great 👍 ) library as well ... |
The whole reason why this is is in a function is probably due to a not so well thought through use case. @MikiDi not sure how you intend to fix it by wrapping it into a closure. I am rather thinking, that initialising the That should fix the race condition, and provides a clear entry point for applications to use other formats. The (open for other ideas) |
The way I understand it, you basically want a module that is configurable "on load", i. e., the ability to specify parameters like Needless to say, both of these methods would change the API in a non-backwards compatible way. So a better solution might indeed be to set up the cache on import and allow for changing it later. This would mean that people who want to use the extended functionality have to jump through one more hoop. Alternatively, you could use a |
I just encountered a bug that was very hard to track down: In a multithreaded environment,
isotime.parse_time()
sometimes fails to parse a valid ISO8601 string of the formHH:MM+HH:MM
. In a console (i.e., in a single-threaded setting), the same string is parsed without error.The problem seems to be in the function
build_time_regexps()
that is called at the beginning ofparse_time()
: It checks whether the global variableTIME_REGEX_CACHE
is empty, and if so, it fills it with various compiled regexes, one after another. If control passes to another thread after some but not all regexes have beenappend()
ed to that list, the other thread will find the list to be non-empty and hence assume it contains all the necessary compiled regexes. Hence it will succeed in parsing some time formats and fail for others.The function
isodates.parse_date()
seems to be using the same technique, and so it likely suffers from the same problem. I have not looked at that function in detail though.The text was updated successfully, but these errors were encountered: