Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.4k
GH-116738: document thread-safety of bisect#136555
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
picnixz left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Possible suggestions:
- Use a
.. note::if you think users should know about it. - Use a
.. warning::if you think users should becareful about it. - Do not use either of them if you think it's just good that users know about it (a note or a warning is quite visible in the sense that it creates a box where the entire content is inside that box).
- Use a subsection about thread-safetiness. Or use a small warning box just saying that the functions are not thread-safe, and add a link to a section at the bottom of the page that contains more detailed explanations.
cc@hugovk
Uh oh!
There was an error while loading.Please reload this page.
Using "note" seems appropriate to me in this case. In the long term, I suggest it makes sense to have a thread-safety (or concurrency-safety) section for any module that has unusual rules (non-atomic functions, shared state). In the "warnings" doc, I added a "Concurrent safety of Context Managers" section to it. That's a case where the rules are quite complex and a dedicated section makes sense. |
| """ | ||
| return all(lst[i - 1] <= lst[i] for i in range(1, len(lst))) | ||
| def run_concurrently(self, worker_func, args, nthreads): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There isthreading_helper.run_concurrently which you can use here
Uh oh!
There was an error while loading.Please reload this page.
I don't think it makes sense to make these functions thread-safe. They are already unsafe when used in the default build. And, given that they can operate on any sequence object, trying to lock the sequence object doesn't make sense.
I added a unit test because I believe these functions should not crash or produce TSAN warnings when they are used on a sequence being mutated in another thread.
📚 Documentation preview 📚:https://cpython-previews--136555.org.readthedocs.build/