I'm not fully convinced this is worth it. I think it's okay for bisect not to be thread safe, but I don't feel very strongly about this one way or the other.

I don't feel strongly about landing it. If we agree current state is good enough, we can go ahead and mark the module as safe and close the PR.

The pure Python implementations in bsiect.py are still not going to be thread safe.

Yes, pure Python version would be different, which is a good reason to consider dropping this change.
Just to clarify my understanding; there is noway to lock the mutex on the object from Python, right?

Just to clarify my understanding; there is noway to lock the mutex on the object from Python, right?

No, we don't expose a way to lock the object. Critical sections are not very useful from Python because the eval breaker checks can run arbitrary code (such as via the GC) and that can temporarily suspend the active critical section. So things would only be "atomic most of the time", which is not a very useful property.

The pure Python implementations in bsiect.py are still not going to be thread safe.

I think this is an issue with several other modules already. For example, the pure-Python implementation ofheapq is not thread-safe while the C implementation is.

I think this is an issue with several other modules already. For example, the pure-Python implementation of heapq is not thread-safe while the C implementation is.

ISTM that all we want in these cases is for the C implementation to not segfault. There is no need to promise more than the pure python version promises. If the heapq module was edited to do more, that should probably be reverted. The C versions are intended to just be accelerators.

I think withheapq the locks were necessary to make it not segfault under race conditions because it directly accessed list internals. I don't think that's the case here because we're using the abstract sequence API instead.

While these functions did not crash before, it is more intuitive for them to lock the sequence they are about to modify.

I think instead of adding locks for this, we should document the thread safety of this module like as in#136555

To clarify, I'm not saying we should necessarily add locking tobisect, I just think we shouldn't reject the idea on the basis that it would create differences between the C and Python implementation, because we already do that in other modules.

There is no need to promise more than the pure python version promises. If the heapq module was edited to do more, that should probably be reverted. The C versions are intended to just be accelerators.

The problem is that by adding memory safety to things like heapq (i.e., making it not crash via locking), we're also incidentally adding thread safety. But, then the Python implementation remains memory safe without thread safety (i.e., doesn't crash but also doesn't really work).

If we make the C implementation thread-safe, users will start writing code that depends on this and won't work with a pure Python implementation.

I think withheapq the locks were necessary to make it not segfault under race conditions because it directly accessed list internals. I don't think that's the case here because we're using the abstract sequence API instead.

Exactly, locks were necessary forheapq because it directly accesses the internals of a list. Without the locks, it would segfault.

I think instead of adding locks for this, we should document the thread safety of this module like as in#136555

I'm closing this PR in favor of documenting it. I believe#136555 will be merged soon.

Thank you all for your reviews and feedback!