Skip to content

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

nascheme
Copy link
Member

@nascheme nascheme commented Jul 11, 2025

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/

@nascheme nascheme added docs Documentation in the Doc dir topic-free-threading labels Jul 11, 2025
@github-project-automation github-project-automation bot moved this to Todo in Docs PRs Jul 11, 2025
@bedevere-app bedevere-app bot added the tests Tests in the Lib/test dir label Jul 11, 2025
Copy link
Member

@picnixz picnixz left a 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 be careful 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

@nascheme
Copy link
Member Author

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.

@nascheme nascheme marked this pull request as ready for review July 14, 2025 18:50
@nascheme nascheme requested a review from rhettinger as a code owner July 14, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review docs Documentation in the Doc dir skip news tests Tests in the Lib/test dir topic-free-threading
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

2 participants